Skip to content

feature(fts): add the full-text search performance test - #15769

Open
m-szymon wants to merge 15 commits into
scylladb:masterfrom
m-szymon:pr/fts-aws-run
Open

feature(fts): add the full-text search performance test#15769
m-szymon wants to merge 15 commits into
scylladb:masterfrom
m-szymon:pr/fts-aws-run

Conversation

@m-szymon

@m-szymon m-szymon commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

ScyllaDB's full-text (BM25) index is served by vector-store, and nothing in SCT
measures it. This adds the benchmark that does: load a document corpus, build a
'fulltext_index' on it, and report index build time, indexing throughput and
query latency to Argus, at several corpus sizes per run.

The orchestration is deliberately workload-agnostic. vector-store serves both a
full-text and a vector index and latte drives both, so what a search benchmark
needs -- a YAML plan of datasets, a dataset as a sequence of shard-loading steps,
an index rebuild and a query phase per step -- is shared in search_perf_test.py,
and fts_test.py is a descriptor naming its rune script, its vocabulary and its
Argus tables. In the follow-ups vector search (or future hybrid search) can become
a second descriptor rather than a second flow.

AWS runs can build vector-store from source over the base VS AMI, to allow testing
pre-release vector-store, and pull their corpora from s3://full-text-search-sct/.
The docker run needs neither: it generates its own tiny corpora and exercises every
branch of the step loop - its purpose is to validate the workflow.

Additional notes

  • Build time is read from vector-store's own log ("starting/finished full scan"),
    which is the only measurement precise enough here: latte's clock has whole-second
    resolution and the index-status endpoint is refreshed on a ~1s ticker, so polling
    either observes each edge late.
  • expected_p99_read_ms is required per query set, on the entry or the dataset's
    defaults. One value selects the Argus table, becomes its P99 validation rule and
    is stated in its description, so the expectation lives in the plan rather than half
    in a label and half in an SCT default nobody set. It is deliberately not a column:
    it is a property of the table, identical on every row.
  • What varies per row is a column, not part of the label -- limit, concurrency,
    rate and an example query. That makes row-label collisions possible, and a colliding
    label is disambiguated by the parameters that differ within its collision group
    rather than by position, so editing a plan does not rename rows other entries
    already report under.
  • The hdr-tag fix is what makes the search phase run at all. latte tags its
    histograms after the rune function, so this test produces fn--search;
    get_latte_operation_type has no search verb either, so it reports the command as
    "user" and the stress_operation in ("WRITE", "READ") fallback does not catch the
    tag -- the detection raises ValueError. That raise happens inside the streaming
    LatteHDRExporter thread, which does not guard split_line, and
    FileFollowerThread.__exit__ re-raises it through future.result() outside the
    try around the command, failing the whole latte invocation.
  • A shard is staged per latte invocation, not shipped with the rune script's
    directory: the full corpora do not fit next to each other on a runner's disk, so a
    shard is fetched, loaded and deleted. extra_files_to_stage on LatteStressThread
    is that hook, and build_stress_cmd is the only point at which it can copy -- the
    RemoteDocker runner is created and destroyed per invocation.
  • The rune scripts are mirrored from scylladb/vector-store's
    latte/full-text-search/, because LatteStressThread resolves .rn paths inside
    the SCT tree. Changes belong upstream first; the copies stay a mirror.

Closes: VECTOR-764

Closes #15697
Closes #15719

Testing

PR pre-checks (self review)

  • I added the relevant backport labels
  • I didn't leave commented-out/debugging code

Reminders

  • Add New configuration option and document them (in sdcm/sct_config.py)
  • Add unit tests to cover my changes (under unit-test/ folder)
  • Update the Readme/doc folder relevant to this change (if needed)

@m-szymon m-szymon added the backport/none Backport is not required label Aug 12, 2026
@m-szymon m-szymon changed the title Pr/fts aws run feature(fts): add the full-text search performance test Aug 12, 2026
@scylladb-promoter

scylladb-promoter commented Aug 12, 2026

Copy link
Copy Markdown
Collaborator

✅ Test Summary: PASSED

✅ Precommit: PASSED

Total Passed Failed Skipped
210 30 0 180

✅ Tests: PASSED

Total Passed Failed Errors Skipped
4973 4942 0 0 31

Full build log

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Adds a reusable search-performance framework and an FTS benchmark covering corpus loading, index builds, query latency, Argus reporting, and optional AWS source builds.

Changes:

  • Adds workload plans, Latte scripts, Docker/AWS configurations, and Jenkins orchestration.
  • Extends vector-store provisioning, polling, metrics, and Argus reporting.
  • Adds unit and integration coverage plus operational documentation.

Reviewed changes

Copilot reviewed 38 out of 39 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
.gitignore Ignores generated FTS corpora.
data_dir/install_vector_store_from_source.sh Builds vector-store from source.
data_dir/latte/fts_search/fts.rn Implements FTS workload phases.
data_dir/latte/fts_search/generate_local_dataset.py Generates local test corpora.
data_dir/latte/fts_search/large_config.yaml Defines full benchmark plan.
data_dir/latte/fts_search/local_config.yaml Defines Docker validation plan.
data_dir/latte/fts_search/metrics.rn Implements relevance metrics.
data_dir/latte/fts_search/sanity_config.yaml Defines default AWS sanity plan.
defaults/test_default.yaml Adds search/source-build defaults.
docs/configuration_options.md Documents new configuration fields.
docs/fts-search-test.md Documents benchmark operation.
fts_test.py Declares the FTS workload.
jenkins-pipelines/performance_staging/fts-search-test.jenkinsfile Adds the performance pipeline.
sdcm/argus_results.py Supports caller-defined result columns.
sdcm/cluster_aws.py Adds AWS source-build provisioning.
sdcm/reporting/tooling_reporter.py Reports source revision metadata.
sdcm/sct_config.py Adds and validates new parameters.
sdcm/stress/latte_thread.py Stages per-invocation files.
sdcm/tester.py Forwards Latte staging files.
sdcm/utils/decorators.py Forwards thresholds and metadata.
sdcm/utils/hdrhistogram.py Recognizes search HDR tags.
sdcm/utils/lint/env_builder.py Maps source-build environment variables.
sdcm/utils/vector_store_client.py Adds index/readiness polling.
sdcm/utils/vector_store_index.py Measures and reports index builds.
sdcm/utils/vector_store_utils.py Resolves and reports source builds.
search_perf_test.py Implements shared search orchestration.
test-cases/fts-search/fts-search-test-docker.yaml Configures local validation.
test-cases/fts-search/fts-search-test.yaml Configures AWS benchmarking.
unit_tests/integration/test_search_perf_test.py Tests the live FTS flow.
unit_tests/integration/test_vector_store.py Tests live index polling and timing.
unit_tests/unit/test_argus_results.py Tests extra Argus columns.
unit_tests/unit/test_fts_test.py Verifies the FTS descriptor.
unit_tests/unit/test_latte_thread.py Tests file staging.
unit_tests/unit/test_search_perf_test.py Tests plan orchestration helpers.
unit_tests/unit/test_tester.py Tests Latte argument forwarding.
unit_tests/unit/test_utils_decorators.py Tests reporting overrides.
unit_tests/unit/test_vector_store_client.py Tests polling behavior.
unit_tests/unit/test_vector_store_index.py Tests build-time parsing.
unit_tests/unit/test_vector_store_source.py Tests source-build handling.
Suppressed comments (1)

data_dir/install_vector_store_from_source.sh:117

  • grep -m1 channel can select a comment or a commented-out alternative before the actual channel = ... assignment—the exact case the preceding comment mentions—causing rustup to receive the wrong value. Match only an anchored TOML assignment and fall back when no assignment exists.
RUST_CHANNEL=$(grep -m1 channel "${CLONE_DIR}/rust-toolchain.toml" 2>/dev/null | cut -d '"' -f 2 || echo "stable")

💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

Comment thread sdcm/sct_config.py Outdated
Comment thread search_perf_test.py Outdated
Comment thread data_dir/latte/fts_search/fts.rn
Comment thread search_perf_test.py
Comment thread search_perf_test.py Outdated
Comment thread search_perf_test.py Outdated
Comment thread data_dir/install_vector_store_from_source.sh Outdated
@m-szymon

Copy link
Copy Markdown
Contributor Author

I publish full feature PR. If it is too big the intermediate PRs are: feature(fts): measure full-text index build time and throughput and feature(fts): add the search phase to the full-text benchmark.
Please review.

It seems I can't trigger claude without write permissions.

@scylladbbot

Copy link
Copy Markdown

@m-szymon new branch manager-3.12 was added, please add backport label if needed

@dimakr

dimakr commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

@claude review

@claude

claude Bot commented Aug 13, 2026

Copy link
Copy Markdown

Claude finished @dimakr's task in 4m 58s —— View job


Code Review

  • Gather context and understand the PR
  • Review core orchestration (search_perf_test.py)
  • Review FTS descriptor (fts_test.py)
  • Review infrastructure changes (sdcm/)
  • Review latte scripts and configs
  • Review unit/integration tests
  • Compile findings and post review

This is a well-structured addition. The separation between the workload-agnostic SearchPerformanceTest and the FTS-specific FtsSearchTest descriptor is clean, the plan-driven approach scales to future workloads, and the test coverage is thorough. The Copilot-flagged issues (qrels tracking, shard dedup, S3 cache key, float validation, AWS gate, retry robustness) were all addressed. The notes below are independent findings.

Findings

Medium — get_status() return type annotation is wrong

VectorStoreClient.get_status() is annotated -> dict, but the unit tests (test_wait_for_ready_accepts_bootstrapping_by_default) mock it to return the plain string "BOOTSTRAPPING". The wait_for_ready logic also relies on status in required_statuses succeeding, which only works if status is a string. The annotation should be -> str.

sdcm/utils/vector_store_client.py:41

Fix this →

Medium — check_interval type hint is int but float is used

wait_for_ready declares check_interval: int = 5, but every call from search_perf_test.py passes VECTOR_STORE_STATUS_POLL_INTERVAL_SECS = 1.0, a float. Mismatched annotations erode trust in the signature. Change to float.

sdcm/utils/vector_store_client.py:90

Medium — Index build outer timeout equals inner timeout (no buffer)

In _build_index, the outer latte timeout is:

duration=_timeout_minutes(max_index_wait)   # e.g. 30 min for 1800s

The rune script also waits max_index_wait seconds internally. If the script's internal wait takes the full budget, the outer SCT timeout can fire at the same moment, creating a race. Adding a buffer (e.g. _timeout_minutes(max_index_wait + 120)) ensures the inner timeout fires first and latte exits cleanly before SCT kills it.

search_perf_test.py:651–655

Fix this →

Low — _count_tsv_lines reads each shard file twice for large corpora

_count_tsv_lines reads the entire shard file to count non-empty lines, then latte reads it again while staging. For shards of hundreds of MB (the full 10M-document dataset), that's a full extra sequential read per shard — across 100 shards, substantial extra I/O. Consider computing the count from the already-staged file via wc -l through the remote runner, or capturing the line count during download using a BufferedReader wrapper. This is a performance note, not a correctness issue.

search_perf_test.py:226–233

Low — O(n²) duplicate detection in _parse_shard_spec

if duplicates := sorted({shard for shard in result if result.count(shard) > 1}):

list.count() inside a set comprehension is O(n) per element, giving O(n²) overall. For a plan with a 0..999 shard spec this is 10⁶ comparisons. Switch to collections.Counter:

counts = Counter(result)
if duplicates := sorted(shard for shard, n in counts.items() if n > 1):

search_perf_test.py:315

Fix this →

Low — rust-toolchain.toml channel extraction may match a comment

At install_vector_store_from_source.sh:123:

RUST_CHANNEL=$(grep -m1 channel "${CLONE_DIR}/rust-toolchain.toml" 2>/dev/null | cut -d '"' -f 2 || echo "stable")

The comment at lines 121–122 says -m1 is there because the file "may mention 'channel' more than once (a comment, a commented out alternative)". But -m1 stops after the first match, which may itself be the commented-out one. Anchor to a real TOML assignment:

RUST_CHANNEL=$(grep -m1 '^channel = ' "${CLONE_DIR}/rust-toolchain.toml" 2>/dev/null | cut -d '"' -f 2 || echo "stable")

data_dir/install_vector_store_from_source.sh:123

Observation — Duplicate term_common entries in large_config.yaml

Step 3 of both datasets has two term_common entries with identical parameters (entries at positions 2 and 4 in the queries list):

- set: term_common         # <-- first unthrottled run
- set: term_common
  limit: 100
- set: term_common         # <-- second unthrottled run, same params

The disambiguation logic handles this correctly (they get run #1 / run #2 suffixes), but it is not obvious whether this is intentional variance sampling or a copy-paste artifact. A comment on the plan entry would make the intent clear.

data_dir/latte/fts_search/large_config.yaml:28–30 (and ~72–74)

Observation — fts.rn returns Ok on failed BM25 query

The search function at fts.rn:269 swallows a failed BM25 request by returning Ok(()). The PR description acknowledges this: the fix belongs in scylladb/vector-store first and will be mirrored. Worth tracking so the copy does not lag behind the upstream fix longer than necessary. This does mean that under error conditions, failed-query latencies contribute to the reported P99 rather than being surfaced as errors.


What is particularly well done

  • Plan-driven validation up front (validate_plan_queries) — catches typos before shards are loaded, not mid-run.
  • Separate shard/query download tracking in _download_dataset_files — correctly handles the case where the same query set appears with and without qrels in different steps.
  • Row label disambiguation in row_labels_for_step — avoids Argus row collisions without making the label depend on the whole group (so adding entries does not rename existing ones).
  • Index build timing from vector-store's own log — higher precision than either latte's clock or the status-endpoint poll, and the reasoning is documented.
  • _checked_* validators — consistent validation of plan-supplied values before they reach shell commands, CQL or S3 paths.
  • Test coverage — unit tests cover the pure helpers thoroughly, and the integration tests validate the full flow against a live Docker cluster.

…ient

'get_index_status' raises on the 404 vector-store returns for an index it has not
discovered yet, which makes "not there yet" indistinguishable from a real error
at the call site. 'get_index_status_or_none' returns None for that one case and
re-raises everything else, so a caller can poll an index across the discovery
gap in either direction -- waiting for a fresh index to appear, or for a dropped
one to disappear again.

'wait_for_index_absent' is that second direction, which every caller recreating
an index needs: vector-store forgets a dropped index asynchronously, so without
the wait a 'CREATE CUSTOM INDEX' can race a drop ScyllaDB already considers done.
A failed request counts as "still there", since mistaking a network hiccup for a
completed drop would let the next build start too early.

'wait_for_ready' grows a 'required_statuses' argument. Its default still accepts
BOOTSTRAPPING, which is what cluster readiness means here -- the service answers
and is catching up -- but a caller that measures index build time or query
latency needs the node actually serving first, and can now ask for that.
Vector-store logs both ends of an index's initial table scan ("starting/finished
full scan on <keyspace>.<index>") with a microsecond tracing timestamp. That scan
is the index build, so those two lines are the most direct and most precise
measurement of it available -- latte's clock has whole-second resolution, and
vector-store's index-status endpoint serves a snapshot refreshed on a ~1s ticker,
so polling either observes each edge late.

'wait_for_index_build_seconds' reads them off the node's log once the build is
over, retrying for a bounded window because the lines reach the runner
asynchronously, and returning None rather than raising when they never arrive:
the index is queryable either way, so a missing measurement should not fail a
run. Two log shapes are handled, since 'BaseNode.system_log' resolves to the raw
tracing line on docker and to the same line behind a log-shipper prefix on aws.

'index_key' folds the case the way ScyllaDB folds unquoted identifiers, which is
how vector-store knows an index -- 'fts_idx_10M_20tok_0' is 'fts_idx_10m_20tok_0'
downstream, and matching without folding silently never matches.

Argus reporting lives here too: 'index_build_columns' and
'send_index_build_result' report build time, how much was indexed and the
throughput that follows from the two. The count column is named by the caller in
its own vocabulary (documents, vectors), and the table itself is declared by the
test that owns it, so its name and description stay next to that test.

None of this is specific to full-text search: it applies to any index
vector-store serves, because the measurement comes from vector-store's own log
rather than from whatever issued the DDL.
…tore

The two helpers of the previous commits are unit tested against mocks and captured
log lines, which leaves exactly the parts that can rot unchecked: whether a
missing index really answers 404, whether a dropped one is really forgotten, and
whether vector-store still writes its "starting/finished full scan" lines in the
shape the parser expects and still reaches the runner's copy of the log.

Two integration tests answer those against a live ScyllaDB and vector-store, using
the docker fixtures the existing vector-store tests already use. Only the
deterministic states are asserted -- before an index exists and after it is
dropped -- rather than racing vector-store's discovery window, which would be
flaky.

They run against a vector index, because that is what the released images support
and it keeps the tests runnable anywhere the other vector-store integration tests
are. Neither helper knows which kind of index it is looking at: the full-text
tests call exactly these two, and this is also the evidence for that claim.
'fts.rn' drives the BM25 workload: the schema and index DDL, the document
load, the index build (which waits for the index to become queryable and
reports the build time and the indexing throughput as latte custom metrics)
and the search phase. 'metrics.rn' holds the IR accuracy metrics -- recall,
precision, NDCG and friends -- the search phase reports against the qrels.

Both are copies of the scripts in scylladb/vector-store's
latte/full-text-search/. They have to live here because 'LatteStressThread'
resolves '.rn' paths inside the SCT tree and copies the containing directory
into the loader container, so a script cannot be loaded from another repo.
Keep them a mirror: changes belong upstream first, and the custom metrics stay
declared even where SCT reads none of them. That is also why the search
function and metrics.rn arrive before anything calls them -- trimming the
scripts to what SCT uses today would only create a divergence to undo.
The orchestration a search benchmark needs is the same whichever index is under
test, because vector-store serves both a full-text and a vector index and latte
drives both: a YAML plan names datasets, a dataset is a sequence of steps, and a
step loads more shards on top of the previous ones and rebuilds the index -- so
one dataset yields results at several corpus sizes. Index build time comes from
vector-store's log and every row is streamed to Argus as it is produced.

What differs between workloads is the rune script, the vocabulary results are
reported in, and the names they are reported under. All of it is declared in a
'SearchWorkload' that a subclass points 'WORKLOAD' at, leaving the subclass with
only its Argus table and its 'test_*' entry point.

'LatteScriptParams' is part of that descriptor because the rune scripts are
mirrored from scylladb/vector-store rather than owned here, and each keeps its
own vocabulary -- the full-text one talks about documents. Mapping those names
per workload means adding a workload never requires renaming a parameter in a
script that lives in another repository.

Getting a shard into the loader container uses the only route the stress
framework offers out of the box: 'LatteStressThread.build_stress_cmd' copies
every top-level file of the rune script's directory in, so the shard is copied
there for the run that loads it and removed afterwards. That is enough for a
corpus small enough to live in the repo and no further -- it routes the data
through the SCT source tree and ships it into every latte invocation of the run
-- so two commits from here it is replaced by a copy straight into the container.
Keeping it this way first is deliberate: the framework change that replaces it is
easier to judge against code that shows what it costs.

The query phase is deliberately absent too. Running a step's query sets and
reporting their latency needs the Argus latency-table extras and the hdr-tag
workload detection fix, so it lands in its own PR on top of this one. What is
here is the half that stands alone: corpus load, index build, and build time and
indexing throughput per step.

Every phase is a single latte command -- one schema change, one shard, one index
build -- so each runs on one loader: 'run_latte_thread' otherwise fans its thread
out to every loader in the set, and a two-loader cluster would load each shard
twice and report a record count, and the indexing throughput derived from it, off
by that factor.

Dataset names have to be distinct within a plan, and the flow says so rather than
finding out later: an index is named after its dataset and step and its build time
is read from the first matching 'full scan' pair in vector-store's log, so a
repeated name would re-report the first dataset's build times against the second
one's record counts.

A dataset whose local directory does not exist is reported as that, because the
corpora are generated rather than tracked -- a fresh clone that skipped the
generator would otherwise fail on an open() of a shard inside a directory the flow
had just created empty.

The first consumer is the next commit, which adds the full-text search test as a
descriptor plus an entry point.
Adds 'fts_test.FtsSearchTest.test_fts_search': load documents into ScyllaDB with
latte, build a 'fulltext_index' on them, and report how long that build took and
at what throughput to Argus.

The test itself is a 'SearchWorkload' descriptor and an entry point, because the
orchestration is the shared search flow of the previous commit. What is declared
here is what makes the benchmark full-text: fts.rn, the '-P' names that script
chose, "docs" as the unit results are counted in, and the Argus names the history
is kept under -- the "FTS Index Build Time" table and the 'fts_idx_*' index names.

What to run is a YAML plan, not a pile of SCT params: 'search_test_config' names
it and has no default, because which datasets and shards to load is the definition
of the test rather than something to fall back on. The param is the shared flow's
rather than this test's -- a vector-search test names its own plan through the same
one -- and it takes a path relative to the SCT root, like every other file a test
case points at. 'local_config.yaml' is the tiny plan the docker run uses, and it
exercises every branch of the step loop -- a single shard, a shard range, an
unsharded corpus and a rebuild that loads nothing.

'generate_local_dataset.py' produces the two corpora that plan names, so a
docker-backend run needs no S3 access. It lives with the plan rather than with the
mirrored rune scripts because the two are one unit: the plan names 'local_tiny'
and 'local_smoke', the generator writes exactly those, and the docs tell you to
run it once. What it writes is gitignored because it is generated, not because it
is temporary -- a run reads the corpora in place and leaves them alone.

'fts-search-test-docker.yaml' runs the whole flow on the docker backend against
those corpora, which is how the orchestration is verified without an AWS cluster.
docs/fts-search-test.md is the operator guide: how to run it, what to check
afterwards, and the plan format. It stays a runbook -- the reasoning behind the
measurement path is here rather than in it.

The query phase follows in its own PR, adding a latency row per query
configuration. The entry point stays as it is when it lands.

The unit tests check the descriptor against the two things it names and cannot
control: fts.rn, which is mirrored from scylladb/vector-store, and the Argus table.
A rename in either would otherwise surface as a latte run silently ignoring every
'-P' it was given, or as a new empty history in Argus.
Until now the search flow got a shard into the loader container by copying it next
to the rune script, because 'LatteStressThread.build_stress_cmd' ships every
top-level file of that directory and nothing else offered a way in. Three things
that cost:

  - the corpus travelled through the SCT source tree, so a real shard would write
    hundreds of megabytes into a git working copy;
  - 'build_stress_cmd' ships *every* top-level file of that directory, so the
    staged shard went into the container of every latte invocation of the run --
    schema, index build, index drop -- not just the load that needed it;
  - a shard had to exist in the tree for the whole run, which rules out fetching
    one, loading it and deleting it before the next. That is the only way a corpus
    too big for the runner's disk can be loaded at all, and what the S3 support
    later builds on.

'LatteStressThread' grows an 'extra_files_to_stage' list of '(local, remote)' pairs
and copies them in 'build_stress_cmd'. That is the only point at which it can: the
'RemoteDocker' command runner is created and destroyed per latte invocation, so
nothing staged earlier would survive to the run, and nothing needs cleaning up
afterwards because the container is the cleanup context.

It belongs on the thread rather than on a subclass in the test. The distinction the
thread already draws is what a script needs on every invocation -- shipped with the
script's directory -- against what only this invocation needs, which had no answer
until now; that is a gap in the stress thread, not something specific to a search
benchmark. 'ClusterTester.run_latte_thread' passes the list through, so a caller
keeps the helper's timeout resolution and its loader fan-out. The load phase in
particular hands latte a cycle count as '-d', which the helper rewrites into a
duration when '--stress-duration' is set, so it has to keep passing an explicit
'duration' to stay out of that branch: easy to miss if a caller reimplemented this,
and silently wrong when missed.

Datasets land under the workload's 'remote_root' inside the container, which is a
new descriptor field.

A unit test can only assert that 'send_files' was called for what it was given.
Whether the file lands where the rune script looks for it depends on the container's
working directory, the '-P' names the script declares and the path the flow
interpolates -- so the integration test stages a corpus, loads it against a live
ScyllaDB and reads the rows back over CQL. Its negative half asserts that an
unstaged corpus loads nothing, which is what keeps the positive half honest.
The per-dataset cycle -- create the schema, load a shard, build the index, read
how long that took, drop it, load the next shard and rebuild -- has until now been
verifiable only by running the whole test on the docker backend and reading the
result tables afterwards. That takes three minutes, a hand-driven hydra
invocation, and a person to interpret it.

This runs the same cycle as a test, in about fifty seconds, and asserts what a run
reports: one build row per step with cumulative record counts and a positive build
time, the rows actually in ScyllaDB, and the last index gone once the dataset is
done.

It runs the flow's own methods rather than a re-implementation of them. Two seams
are replaced: '_run_latte', which would otherwise need ClusterTester's loader
provisioning, and '_report_build_metrics', so the rows can be asserted instead of
submitted to Argus. Everything between those -- the step loop, the '-P' mapping,
the index naming, reading the build time out of vector-store's log, waiting for
the drop to be noticed -- is the code that ships.

It needs a ScyllaDB with a full-text index and a vector-store that serves one, and
neither is released yet, so it skips unless the images are present locally. The
check is a collection-time 'skipif' rather than a 'skip' in the body, because the
fixture that starts the container would otherwise try to pull a missing image
during setup, and that is an error rather than a skip. Both images are overridable
by environment variable, so this keeps working as the released ones catch up.
… table

Two opt-in parameters on 'latency_calculator_decorator', both threaded into
'send_result_to_argus' and both inert for every existing caller:

'extra_columns' appends 'ColumnMetadata' to the result table's schema, and the
decorated function may then return an 'extra_values' dict written once per row
alongside the usual latency cells. This is for per-row metadata that belongs
next to the numbers rather than folded into an ever-longer row label -- the FTS
test uses it to report the query configuration and an example query. The columns
are appended to a copy, since 'Meta.Columns' is a class attribute shared by every
test that reports a latency table.

'error_thresholds' overrides the 'latency_decorator_error_thresholds' test param
for one call, for a caller whose expected latency is a property of what it is
running rather than of the test as a whole -- again FTS, where the expectation
comes from the dataset plan and differs per query set.

'extra_values' is only emitted for a result with a single HDR tag, the same
branch that writes 'duration' and 'start time': with several tags there is one
row per tag and no per-tag value to write.
'_HdrRangeHistogramBuilder' infers the workload from the hdr tag name and had
no keyword matching a search operation. For a latte 'search' function that is
not a cosmetic mislabel, it is fatal: 'get_latte_operation_type()' has no
'search' verb either, so it classifies 'latte run -f search' as 'user', which
means the 'stress_operation in ("WRITE", "READ")' fallback does not apply and
'_get_workload_type_by_hdr_tag()' raises ValueError on the 'fn--search' tag.

That raise lands in the streaming 'LatteHDRExporter' thread, which does not
guard 'split_line()', and 'FileFollowerThread.__exit__' re-raises it through
'future.result()' outside the 'try' that 'LatteStressThread._run_stress' wraps
around the command -- so the latte invocation fails rather than just losing a
Grafana label. Only the 'latency_calculator_decorator' path is safe, because
there 'stress_operation' is the caller's 'workload_type'.

Add 'search' next to the other read verbs.
Completes the flow: after a step has loaded its shards and built its index, run
the query sets the plan lists against that index and report their latency to
Argus. The two framework changes this needs are the previous two commits -- the
Argus latency-table extras and the hdr-tag workload detection.

Every query entry must resolve an 'expected_p99_read_ms', on the entry itself or
the dataset's 'defaults'. That single value selects the Argus table, becomes its
P99 validation rule and is stated in its description, so the expectation lives in
the plan rather than half in a label and half in an SCT default nobody set. It is
deliberately not a column: it is a property of the table, identical on every row.

What does vary per row is reported as columns -- limit, concurrency, rate and an
example query -- which is why row labels carry only dataset, record count, step
ordinal and query set. The step ordinal is there for the same reason the build
row already carries one: the record count does not identify a step, because a
step with an empty 'shards' list loads nothing and so repeats its predecessor's
count. Without it, a query set run again on a rebuild would land on the row the
earlier step reported under. It also lines a query row up with its build row.

That leaves collisions within one step: two entries for the same set differing
only in parameters that are now columns would land on one row coordinate, and
Argus would keep whichever value was submitted last. 'row_labels_for_step'
suffixes a colliding label with the query configuration, and additionally numbers
the entries that are identical in every parameter. The suffix names every
parameter rather than only those that differ within the collision group: the
shorter form depends on the whole group, so adding one entry varying a parameter
the others agreed on would rewrite the label of every one of them and lose their
Argus history. As written it depends on nothing but the entry, so reordering
never renames anything and adding an entry only affects rows sharing its label.

The plan's query entries are resolved once, up front, rather than at the point
each query runs. Every check here can raise -- an unknown parameter type, a
duration latte cannot parse, a missing expected latency -- and by the time a
query runs, its step has already loaded its shards and built its index, which on
a real corpus is tens of minutes spent to report a typo. Nothing in the pass
touches the cluster or the dataset files.

limit, concurrency, rate and duration are checked rather than merely read,
because all four are interpolated into a latte command line -- the same reason
'_checked_name' exists for the plan's names. A duration must be latte's time form
specifically: 'get_timeout_from_stress_cmd' parses no other, and one it cannot
read silently gives the search phase the whole 'test_duration' as its timeout.

The workload descriptor grows the four rune parameters and the two Argus names
the query phase needs. Its fields are required, so fts_test.py's instance is
updated here rather than in the next commit -- without it the module would not
import.
Turns on the query phase for the full-text test: the docker plan now names query
sets, so a run reports a latency row per query configuration alongside the index
build rows.

'local_config.yaml' exercises the parts of the reporting that are easy to get
wrong, and covers both ways two query entries can land on one Argus row.
'local_tiny' asks for a rate-limited query set on its first step, and on its
second repeats one query configuration verbatim -- the two entries resolve to the
same set, table and parameters within a single step, so they must come out with
' run scylladb#1' and ' run scylladb#2' appended to their configuration suffix. 'local_smoke'
covers the other way: it carries qrels, which turns on the rune script's
relevance metrics, then a step that neither loads nor queries, then a second
no-load rebuild repeating the first step's query set. Nothing is loaded between
the three, so all of them report the same document count and only the 'step #N'
component of the row label keeps the last one off the first one's row.

The test case documents why 'use_hdrhistogram: true' is required now:
'latency_calculator_decorator' returns early without it, so the latency tables
would silently not appear.

docs/fts-search-test.md gains what a plan author needs -- that every query set has
to state an expected P99 and what that value selects, and how repeated query
configurations are told apart -- plus the latency tables in the post-run checklist.
It stays a runbook: the reasoning behind the label scheme is in this message, not
in it.

It also loses the trailing blank line it was added with, which 'end-of-file-fixer'
rejects. The line predates this branch, but the hook runs on every file a PR
touches, so it would fail CI here.
Released vector-store lags behind master, so testing a feature that is not
in a release -- full-text indexes, say -- meant waiting for one or hand
pinning an AMI. Add a source build over the base VS AMI instead.

It is requested by setting 'vector_store_source_repo' or
'vector_store_source_ref'; whichever is omitted falls back to
scylladb/vector-store and 'master', so a single param is enough for both the
common cases (a branch of upstream, or a fork's default branch). Both are
kept out of 'defaults/test_default.yaml' as empty on purpose -- that is what
makes "either one is set" a usable signal. A source build is mutually
exclusive with 'vector_store_version' and only works on aws; both are
rejected in the config, before any AMI lookup, so a misconfiguration reads
as one instead of as a cloud API error.

The base AMI now only has to supply the right architecture, since the build
replaces the binary it ships, so it defaults to the newest VS AMI for the
region rather than having to be pinned per test case.

The installer is vendored into SCT rather than fetched from the ref being
built, so refs predating it upstream can be built too. It leaves a
'.source-commit' marker behind, which the version reporter reads to record
the repo, ref and exact SHA in Argus -- a source build's 'git describe'
version does not identify what was tested on its own.

Also derive the install dir from 'ami_vector_store_user' instead of
hardcoding /home/ubuntu next to a chown that used the param, so the .env
path, the build's install dir and the unit's WorkingDirectory cannot drift.
The docker run reads its tiny corpus from the repo, but a real run cannot:
the 10M-document datasets are hundreds of MB per shard and live in
s3://full-text-search-sct/. This teaches the test to fetch them.

A dataset in a plan may now carry a 'base_url'; its shard, query and qrels
files are pulled from there. The query and qrels files are fetched once per
dataset, the shards one at a time, right before the latte invocation that
loads them -- and deleted again once loaded, because the full corpora do not
fit next to each other on a runner's disk. Only what this run downloaded is
deleted: a locally supplied dataset is the input rather than a cache, so a
'base_url'-less plan keeps working across runs.

'search_test_config' additionally accepts a full 's3://bucket/key' URL, so a run
can use a plan that is not in the repo at all -- one overridable string
(SCT_SEARCH_TEST_CONFIG) rather than an edit to the nested 'download_from_s3'
mapping, which the environment cannot override. Downloaded plans land in the
gitignored data_dir/latte/fts_search/downloaded/ so they are not copied into
every loader container along with the rune scripts. 'download_from_s3' is
honoured too, for anything the plan itself does not name.

Both the shard and the plan transfers ask for the same retry behaviour
'sdcm.utils.common.S3Storage' uses rather than the botocore defaults, since
they run over the whole length of a multi-hour run.
The real run: 'fts-search-test.yaml' provisions an i4i.xlarge db, a c5.large
loader and an r8g.xlarge vector-store built from source ('master', since no
released vector-store has full-text index support yet), and pulls its corpora
from s3://full-text-search-sct/.

Two plans come with it. 'sanity_config.yaml' is the default: 5 of the 100
shards of the 10M/20-token dataset over two load steps, then five no-load
rebuilds on that same corpus to sample index build time variance in isolation
from load time. Enough to answer "does this build work end to end?" without
committing to the full sweep. 'large_config.yaml' is that sweep: both
10M-document datasets in full, three load steps each, selected with
SCT_SEARCH_TEST_CONFIG.

The jenkinsfile wires the job up on aws/us-east-1 so runs land in Argus
properly. perfRegressionParallelPipeline has no vector-store parameter and no
test_duration, so from Argus every knob this test cares about -- which fork or
ref to build, which plan to run, how long to let it run -- goes through the
generic 'extra_environment_variables' field. The docs spell that out rather
than leaving a reader looking for a dropdown that does not exist.
@m-szymon

Copy link
Copy Markdown
Contributor Author

Claude finished @dimakr's task

All minor findings, but I fixed most of them.
Only "_count_tsv_lines reads each shard file twice" is hard to fix here - probably requires some better support in latte and it something to look at in follow-up tasks.

@m-szymon

Copy link
Copy Markdown
Contributor Author

Please review (or at least partial PR #15697).

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

Labels

area/vector backport/none Backport is not required P3 Medium Priority symptom/quality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants