Skip to content

[benchmarker] Add benchmarker configuration and report definitions#1567

Open
BenjaminPelletier wants to merge 1 commit into
interuss:mainfrom
BenjaminPelletier:benchmarker-config
Open

[benchmarker] Add benchmarker configuration and report definitions#1567
BenjaminPelletier wants to merge 1 commit into
interuss:mainfrom
BenjaminPelletier:benchmarker-config

Conversation

@BenjaminPelletier

Copy link
Copy Markdown
Member

This PR adds the schemas for inputs (configuration) to a new benchmarker tool described in #1566 as well as outputs (report). The top level input is BenchmarkConfiguration: it describes a sequence of scenarios to run using loads composed of virtual users, some of which use uss_qualifier resources, bookended by setup and teardown actions, plus artifacts to generate from the data gathered. isas_uncontended.jsonnet provides a concrete example (verified with an initial implementation) of virtual flight planner users who create and delete ISAs at a particular location. It produces a series of graphs for different internode latencies between DSS instances, each showing the throughput vs load graph for each of the three DSS instances. The top level output is BenchmarkRunReport, similar to uss_qualifier's TestRunReport.

@BenjaminPelletier BenjaminPelletier marked this pull request as ready for review July 13, 2026 07:02
},
}
for latency_ms in latencies_ms
] + [

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if it's already correct based on just configuration, but 'make clean-locally' should be run after each 'test' on each 'latencies', to avoid current state of crdb "leaking" between 'test'.

('test' = measure on one specific latency there).

} for latency_ms in latencies_ms
],
},
},

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be possible to show with that:

  • average between dss ?
  • latency / load_factor swapped ?

(as examples in the file)

from monitoring.benchmarker.configurations.loads import BenchmarkLoadName


class BenchmarkScenarioName(str):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit/general meta question unrelated to the PR: why use a specific type instead of just str?


Benchmark configuration must contain a `resources.astm.f3548.v21.DSSInstanceResource` resource with each of these names."""

dss_selection_strategy: Optional[ASTMDSSSelectionStrategy]

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the default? Because I think we want random in most of the cased to avoid issue with CRDB placement that can be random.


class RawReportSpecification(ImplicitDict):
name: str
"""Machine-level name for this figure. Used as the output file name."""

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"""Machine-level name for this figure. Used as the output file name."""
"""Machine-level name for this report. Used as the output file name."""

from monitoring.uss_qualifier.resources.definitions import ResourceCollection


class BenchmarkConfiguration(ImplicitDict):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Should we have a name there for identification of various config?

class ThroughputStabilityCriteria(ImplicitDict):
"""Criteria used to determine when it is valid to start collecting throughput data in a step.

Any specified field that evaluates to false will cause this criteria to evalute to false"""

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Any specified field that evaluates to false will cause this criteria to evalute to false"""
Any specified field that evaluates to false will cause this criteria to evaluate to false"""

any_of: Optional[list[StepCompletionCriteria]]

sampling_duration_at_least: Optional[StringBasedTimeDelta]
"""Evalutes true when the step has been collecting valid throughput data for at least this long."""

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"""Evalutes true when the step has been collecting valid throughput data for at least this long."""
"""Evaluates true when the step has been collecting valid throughput data for at least this long."""

any_of: Optional[list[LoadCompletionCriteria]]

throughput_lower_than_peak: Optional[ThroughputPastPeak]
"""Evalutes true when the throughput of the specified operations for the most recently-completed step drops below the specified fraction of the maximum throughput of all prior steps."""

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"""Evalutes true when the throughput of the specified operations for the most recently-completed step drops below the specified fraction of the maximum throughput of all prior steps."""
"""Evaluates true when the throughput of the specified operations for the most recently-completed step drops below the specified fraction of the maximum throughput of all prior steps."""

flux to remain in steady-state after `end_time` until completion of the last operation started before
`end_time` for the throughput calculation to be valid. Instead, the partial work of operations in progress
at `end_time` effectively discarded by this approach should be (statistically) exactly balanced by the
partial work included "for free" of operations started before `start_time` that end within the time window.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That fine if the "load" is not cut at the end of the test (witch locust unfortunately do.), will it be correct there?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

2 participants