Add the DuckDB execution engine - #25
Open
dionmcm wants to merge 1 commit into
Open
Conversation
Selected with rvf.execution.engine=duckdb against the seam in the previous commit. No MySQL, no JPA, no datasource: a validation runs against a DuckDB file database built from the release under test, and the assertion SQL comes from a precompiled store rather than from the assertion tables. The engine, in org.ihtsdo.rvf.core.service.duck: * DuckStore / DuckStoreLocator - read the precompiled store, and refuse a store that was not published from the corpus in this artefact. * DuckBinder - sentinel substitution, mirroring MySqlQueryTransformer, including its rule that a statement still holding an unbound <PREVIOUS>/<DEPENDENCY> is dropped per statement rather than per assertion. * DuckReleaseUnpacker / DuckMaterialiser - unpack an RF2 archive and materialise its files as tables. * DuckDbAssertionExecutionService - prepareSchema() then execute(assertions). * DuckDbValidationService - the SqlAssertionValidationService implementation, and the resource/assertion phase ordering the MySQL service also relies on. * DuckAssertionService / DuckAssertionSource - supply the AssertionService interface from the store, which is what keeps the submission REST surface working without the assertion tables. * DuckFailuresExtractor - the whitelist and module filtering, from qa_result. Why the store is precompiled and checked in rather than generated at build time: transpiling MySQL to DuckDB is the only part of this that needs Python, and doing it at runtime would put a Python toolchain and a sqlglot version inside the server. Precompiling makes the dialect a reviewable, diffable, version-stamped artefact instead of something regenerated invisibly on every run. BundledStoreMatchesCorpusTest is what stops that becoming a silent trap, and it earned its place during this carve: the store shipped here was published from corpus 0160dd2, and IHTSDO's develop had moved one commit ahead to 694b06d (VAL-440), which rewrote exactly 20 release-type scripts. The test failed with '20 of 360 assertions differ' rather than letting a run execute the old SQL and report it under the new assertion text. The store in this commit is republished from 694b06d, and only the assertions section changed - ports and prerequisites are byte-identical. duckdb_jdbc is pinned to 1.5.5.1 to match the sqlglot dialect the store was transpiled against; a different minor can parse the same SQL differently. rvf.duck.threads defaults to Runtime.availableProcessors() because DuckDB sizes its own pool from the machine and ignores affinity and cgroup quotas - measured here, under taskset -c 0-1 on a 10-core host DuckDB reports threads=10 while the JVM correctly reports 2, and a container CPU limit is worse: 64 threads inside a two-core quota. Bounding it correctly is worth 2.6x at one core. 87 tests added, including a probe harness that runs a real release end to end. Full suite: 328 run, 0 failures, 142 errors, 21 skipped - the same 142 pre-existing Testcontainers-without-Docker errors as clean develop.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The engine, selected with
rvf.execution.engine=duckdbagainst the seam in #24. No MySQL, no JPA, no datasource: a validation runs against a DuckDB file database built from the release under test, with assertion SQL from a precompiled store.Review order: #23, #24, then this.
Classes
DuckStore/DuckStoreLocatorDuckBinderMySqlQueryTransformer, including the per-statement drop for unbound<PREVIOUS>/<DEPENDENCY>DuckReleaseUnpacker/DuckMaterialiserDuckDbAssertionExecutionServiceprepareSchema()thenexecute(assertions)DuckDbValidationServiceSqlAssertionValidationServiceimplementation, and the resource/assertion phase ordering the MySQL service also needsDuckAssertionService/DuckAssertionSourceAssertionServiceinterface from the store, which keeps the submission REST surface working without assertion tablesDuckFailuresExtractorqa_resultWhy the store is precompiled and checked in
Transpiling MySQL to DuckDB is the only part that needs Python. Doing it at runtime would put a Python toolchain and a
sqlglotversion inside the server. Precompiling makes the dialect a diffable, version-stamped artefact.duck/README.mdhas the republish procedure.duckdb_jdbcis pinned to 1.5.5.1 to match thesqlglotdialect the store was transpiled against.BundledStoreMatchesCorpusTestA store published from a different corpus than the one shipped beside it would execute old SQL and report it under new assertion text and uuids — a complete, plausible, wrong report.
It fired during this work. The store came from corpus
0160dd2;develop's corpus had moved one commit to694b06d(VAL-440), rewriting 20 release-type scripts. The store here is republished from694b06d; only theassertionssection changed, ports and prerequisites are byte-identical.Consequence for you:
checkout-resources.shclones the corpus unpinned (--branch develop) and the pom runs it during the build, so this store needs republishing whenever the corpus moves. Pinning that clone is a separate question.One measured defect
DuckDB sizes its thread pool from the machine and ignores CPU affinity and cgroup quotas. On a 10-core host under
taskset -c 0-1,nprocand the JVM report 2; DuckDB reports 10. A pod withlimits.cpu: 2on a 64-core node would run 64 threads inside a two-core quota. Bounding it is worth 2.6x at one core (674s to 259s on a full edition).rvf.duck.threadstherefore defaults toRuntime.availableProcessors(), overridable for benchmarking.rvf.duck.memory.limitis unset by default.Verification
87 tests added; the same 142 pre-existing errors (Testcontainers, no Docker daemon).
Also run end to end on a real 894MB edition through the REST API with no MySQL present:
POST /run-post-> 201, worker consumes,GET /result/{runId}-> 191 tests run, 58 failures, 1 warning, 0 skips, 54 incomplete, in 194s. The 54 incomplete are the expected result of supplying no previous or dependency release.