feat: re-pin to qbit-build-parent 2.0.0, fix index drift, add reconcile (QRun-IO/qbit-quick-search#6) #7

Merged
KofTwentyTwo merged 9 commits from feature/GH-6-repin-and-drift-fix into develop 2026-09-26 01:36:57 +00:00
KofTwentyTwo commented 2026-09-26 00:52:42 +00:00 (Migrated from github.com)

Summary

Re-pins qbit-quick-search to qbit-build-parent 2.0.0 (qqq 4.0.0) and verifies it on qqq 4.1.0-SNAPSHOT. It also fixes index drift and adds a reconcile process, so qqq-all can include this qbit (plan task B1).

The integration tests used to skip silently: Testcontainers 1.21.0 cannot talk to current Docker Engines. Once they actually ran, they showed the QBit could not reach OpenSearch at all on the qqq 4.0 BOM. That is fixed here too (second bullet below).

Closes #6
Part of QRun-IO/qqq-all#1

Changes

  • Re-pin (build): parent is now com.kingsrook:qbit-build-parent:2.0.0. The child's qqq-bom-pom import is gone, so the qqq version comes only from the parent (ADR-0007). The parent has no qqq-version property, so -Dqqq.version=... activates an opt-in qqq-version-override profile that imports that qqq-bom-pom version. Default builds never activate it. Java 21 and the Apache-2.0 <licenses> block are unchanged. Tests now use QInstance.withInstanceDefaultAuthentication (qqq 4.0 removed setAuthentication).
  • OpenSearch client on the qqq 4.0 BOM (fix):
    • The child pinned httpclient5 5.3, but the BOM brings httpcore5 5.4.3. That combination fails with NoSuchMethodError in the IO reactor, so every OpenSearch call failed. httpclient5 and jackson-datatype-jsr310 are now BOM-managed (5.6.3 / 2.21.5).
    • httpclient5 5.6 also decompresses gzip responses itself but keeps the Content-Encoding header, so opensearch-java 2.10 gunzipped the body again ("Not in GZIP format"). The client builder now calls disableContentCompression().
  • ITs fail instead of skip on CI (test):
    • @Testcontainers(disabledWithoutDocker = true) is replaced by RequiresDockerCondition. Without Docker it skips locally but throws when CI=true, so the IT classes fail.
    • Testcontainers is bumped to 1.21.4 (the release for current Docker Engine APIs; 1.21.0 gets HTTP 400 from Docker 29).
  • Index drift on update and delete (fix):
    • Update: QuickSearchPostUpdateCustomizer re-reads the updated records from the source table (by primary key, in the update's transaction) and indexes the full records. Before, it indexed the sparse update record, which dropped unchanged fields from the document. Records with errors are skipped.
    • Delete: QuickSearchPostDeleteCustomizer skips records with errors or no primary key. SynchronousIndexEventPublisher now bulk-deletes through the new QuickSearchOpenSearchClient.deleteDocuments. Before, it deleted one document at a time, and an exception stopped the remaining deletes while the customizer swallowed it. Documents already missing count as deleted.
    • Failures: per-document bulk failures, for index and delete, now raise a QException naming the documents; they used to be discarded. The customizers still catch it and log a warning, so writes are never blocked.
  • Reconcile process (feat): new quickSearchReconcileIndex process (ReconcileIndexStep), registered with the other processes and in the admin app, with optional tableName input.
    • It re-indexes every source record, paging in primary-key order (keyset, ordered), so no row is skipped or read twice.
    • If every document indexed, it refreshes the index and deletes that table's documents with indexedAt before the run started (or no indexedAt). Those are orphans from missed deletes.
    • It never wipes the table first, so search keeps working. If any document fails, it skips the purge and marks the run FAILED.
    • It records a RECONCILE run, updates the index row, and outputs recordsIndexed / documentsRemoved.
  • Repo cleanup (chore): removed the stray root com/ directory (AGPL-headered copies of qqq core classes, outside the source tree) and .claude/settings.local.json, and added .claude/settings.local.json to .gitignore.
  • Docs: README (reconcile usage, process table, version override, CI test policy) and CLAUDE.md (stale version, test count, and dependency facts).

main has no commits that are not in develop (git rev-list --count origin/develop..origin/main = 0), so no back-merge PR was needed.

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (consumers now need qqq 4.x; the qbit's own API is additive)
  • Documentation update
  • Refactoring (no functional changes)
  • Dependencies update

Testing

Commands, run in a fresh clone (Java 21.0.12, Maven 3.9.16, Docker 29.8.0). mvn install was never run.

Command Result
mvn -B verify on untouched develop (baseline) BUILD SUCCESS, 215 unit tests; all 9 ITs skipped ("Could not find a valid Docker environment")
mvn -B clean verify at cf3bbcb (qqq 4.0.0 from the parent) BUILD SUCCESS: 233 unit tests, 12 ITs, 0 failures, 0 skipped
mvn -B -U -Dmaven.repo.local=/tmp/b1-m2-snapshot -Dqqq.version=4.1.0-SNAPSHOT clean verify at cf3bbcb BUILD SUCCESS: 233 unit tests, 12 ITs, 0 failures, 0 skipped; resolved qqq-backend-core-4.1.0-20260926.003607-7 from central.sonatype.com snapshots
No-Docker check: docker run maven:3.9-eclipse-temurin-21 (no Docker socket) with mvn -B verify CI unset: 9 ITs skipped, BUILD SUCCESS. CI=true: both IT classes fail with "Docker is required for integration tests when CI=true", BUILD FAILURE (exit 1)
mvn help:effective-pom with and without -Dqqq.version=4.1.0-SNAPSHOT qqq-backend-core 4.0.0 by default, 4.1.0-SNAPSHOT with the override; license is Apache-2.0

The 4.1.0-SNAPSHOT run used an isolated local repository because ~/.m2 already held a locally installed 4.1.0-SNAPSHOT; this way the gate tests the published snapshot.

Red-first evidence for the drift fix: with the old QuickSearchPostUpdateCustomizer restored, IndexDriftIntegrationTest.testPartialUpdate_unchangedFieldsStaySearchable fails (the unchanged city is no longer searchable); with the fix it passes.

New tests:

  • RequiresDockerConditionTest

  • ReconcileIndexStepTest (6)

  • publisher, client, and customizer tests for bulk delete, failure reporting, source re-read, and error or no-key skips

  • IndexDriftIntegrationTest (3, against real OpenSearch 2.11): partial update, delete with a missing document in the batch, and reconcile repairing an orphan, a missing document, and a stale document

  • Unit tests pass (mvn test)

  • Integration tests pass (mvn verify)

  • Manual testing performed

Checklist

  • My code follows the project's style guidelines
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings (Maven model warnings are the same as on develop)
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

Additional Notes

  • CI will now run the ITs. The orb's mvn_test_only job uses a machine executor with Docker, and CircleCI sets CI=true, so the 12 ITs run there, and a CI build without Docker now fails instead of passing.
  • Parent license metadata: the published qbit-build-parent 2.0.0 POM still declares AGPL-3.0 <licenses>. This child declares Apache-2.0, which overrides it in the effective POM, but the parent itself needs a follow-up per ADR-0007.
  • JaCoCo gate is still a no-op: jacoco:check skips because no prepare-agent execution is bound, so there is no jacoco.exec. This predates the PR and is left alone to keep scope.
  • Reconcile cutoff: the cutoff and indexedAt values come from application clocks, so nodes are assumed to keep their clocks in sync.
  • Update cost: each update now runs one extra primary-key query (the same pattern UpdateAction uses to pre-fetch old records).
  • Not done here (outside B1): scheduling basepull/reconcile, permission filtering on search results, and replacing the full reindex's wipe-then-rebuild.

🤖 Generated with Claude Code

## Summary Re-pins `qbit-quick-search` to `qbit-build-parent` 2.0.0 (qqq 4.0.0) and verifies it on qqq 4.1.0-SNAPSHOT. It also fixes index drift and adds a reconcile process, so qqq-all can include this qbit (plan task B1). The integration tests used to skip silently: Testcontainers 1.21.0 cannot talk to current Docker Engines. Once they actually ran, they showed the QBit could not reach OpenSearch at all on the qqq 4.0 BOM. That is fixed here too (second bullet below). Closes #6 Part of QRun-IO/qqq-all#1 ## Changes - **Re-pin (build):** parent is now `com.kingsrook:qbit-build-parent:2.0.0`. The child's `qqq-bom-pom` import is gone, so the qqq version comes only from the parent (ADR-0007). The parent has no qqq-version property, so `-Dqqq.version=...` activates an opt-in `qqq-version-override` profile that imports that `qqq-bom-pom` version. Default builds never activate it. Java 21 and the Apache-2.0 `<licenses>` block are unchanged. Tests now use `QInstance.withInstanceDefaultAuthentication` (qqq 4.0 removed `setAuthentication`). - **OpenSearch client on the qqq 4.0 BOM (fix):** - The child pinned `httpclient5` 5.3, but the BOM brings `httpcore5` 5.4.3. That combination fails with `NoSuchMethodError` in the IO reactor, so every OpenSearch call failed. `httpclient5` and `jackson-datatype-jsr310` are now BOM-managed (5.6.3 / 2.21.5). - httpclient5 5.6 also decompresses gzip responses itself but keeps the `Content-Encoding` header, so opensearch-java 2.10 gunzipped the body again ("Not in GZIP format"). The client builder now calls `disableContentCompression()`. - **ITs fail instead of skip on CI (test):** - `@Testcontainers(disabledWithoutDocker = true)` is replaced by `RequiresDockerCondition`. Without Docker it skips locally but throws when `CI=true`, so the IT classes fail. - Testcontainers is bumped to 1.21.4 (the release for current Docker Engine APIs; 1.21.0 gets HTTP 400 from Docker 29). - **Index drift on update and delete (fix):** - Update: `QuickSearchPostUpdateCustomizer` re-reads the updated records from the source table (by primary key, in the update's transaction) and indexes the full records. Before, it indexed the sparse update record, which dropped unchanged fields from the document. Records with errors are skipped. - Delete: `QuickSearchPostDeleteCustomizer` skips records with errors or no primary key. `SynchronousIndexEventPublisher` now bulk-deletes through the new `QuickSearchOpenSearchClient.deleteDocuments`. Before, it deleted one document at a time, and an exception stopped the remaining deletes while the customizer swallowed it. Documents already missing count as deleted. - Failures: per-document bulk failures, for index and delete, now raise a `QException` naming the documents; they used to be discarded. The customizers still catch it and log a warning, so writes are never blocked. - **Reconcile process (feat):** new `quickSearchReconcileIndex` process (`ReconcileIndexStep`), registered with the other processes and in the admin app, with optional `tableName` input. - It re-indexes every source record, paging in primary-key order (keyset, ordered), so no row is skipped or read twice. - If every document indexed, it refreshes the index and deletes that table's documents with `indexedAt` before the run started (or no `indexedAt`). Those are orphans from missed deletes. - It never wipes the table first, so search keeps working. If any document fails, it skips the purge and marks the run `FAILED`. - It records a `RECONCILE` run, updates the index row, and outputs `recordsIndexed` / `documentsRemoved`. - **Repo cleanup (chore):** removed the stray root `com/` directory (AGPL-headered copies of qqq core classes, outside the source tree) and `.claude/settings.local.json`, and added `.claude/settings.local.json` to `.gitignore`. - **Docs:** README (reconcile usage, process table, version override, CI test policy) and CLAUDE.md (stale version, test count, and dependency facts). `main` has no commits that are not in `develop` (`git rev-list --count origin/develop..origin/main` = 0), so no back-merge PR was needed. ## Type of Change - [x] Bug fix (non-breaking change that fixes an issue) - [x] New feature (non-breaking change that adds functionality) - [x] Breaking change (consumers now need qqq 4.x; the qbit's own API is additive) - [x] Documentation update - [ ] Refactoring (no functional changes) - [x] Dependencies update ## Testing Commands, run in a fresh clone (Java 21.0.12, Maven 3.9.16, Docker 29.8.0). `mvn install` was never run. | Command | Result | |---|---| | `mvn -B verify` on untouched `develop` (baseline) | BUILD SUCCESS, 215 unit tests; **all 9 ITs skipped** ("Could not find a valid Docker environment") | | `mvn -B clean verify` at `cf3bbcb` (qqq 4.0.0 from the parent) | **BUILD SUCCESS**: 233 unit tests, 12 ITs, 0 failures, 0 skipped | | `mvn -B -U -Dmaven.repo.local=/tmp/b1-m2-snapshot -Dqqq.version=4.1.0-SNAPSHOT clean verify` at `cf3bbcb` | **BUILD SUCCESS**: 233 unit tests, 12 ITs, 0 failures, 0 skipped; resolved `qqq-backend-core-4.1.0-20260926.003607-7` from central.sonatype.com snapshots | | No-Docker check: `docker run maven:3.9-eclipse-temurin-21` (no Docker socket) with `mvn -B verify` | CI unset: 9 ITs skipped, BUILD SUCCESS. `CI=true`: both IT classes fail with "Docker is required for integration tests when CI=true", BUILD FAILURE (exit 1) | | `mvn help:effective-pom` with and without `-Dqqq.version=4.1.0-SNAPSHOT` | `qqq-backend-core` 4.0.0 by default, 4.1.0-SNAPSHOT with the override; license is Apache-2.0 | The 4.1.0-SNAPSHOT run used an isolated local repository because `~/.m2` already held a locally installed 4.1.0-SNAPSHOT; this way the gate tests the published snapshot. Red-first evidence for the drift fix: with the old `QuickSearchPostUpdateCustomizer` restored, `IndexDriftIntegrationTest.testPartialUpdate_unchangedFieldsStaySearchable` fails (the unchanged `city` is no longer searchable); with the fix it passes. New tests: - `RequiresDockerConditionTest` - `ReconcileIndexStepTest` (6) - publisher, client, and customizer tests for bulk delete, failure reporting, source re-read, and error or no-key skips - `IndexDriftIntegrationTest` (3, against real OpenSearch 2.11): partial update, delete with a missing document in the batch, and reconcile repairing an orphan, a missing document, and a stale document - [x] Unit tests pass (`mvn test`) - [x] Integration tests pass (`mvn verify`) - [ ] Manual testing performed ## Checklist - [x] My code follows the project's style guidelines - [x] I have performed a self-review of my own code - [x] I have commented my code, particularly in hard-to-understand areas - [x] I have made corresponding changes to the documentation - [x] My changes generate no new warnings (Maven model warnings are the same as on `develop`) - [x] I have added tests that prove my fix is effective or that my feature works - [x] New and existing unit tests pass locally with my changes ## Additional Notes - **CI will now run the ITs.** The orb's `mvn_test_only` job uses a machine executor with Docker, and CircleCI sets `CI=true`, so the 12 ITs run there, and a CI build without Docker now fails instead of passing. - **Parent license metadata:** the published `qbit-build-parent` 2.0.0 POM still declares AGPL-3.0 `<licenses>`. This child declares Apache-2.0, which overrides it in the effective POM, but the parent itself needs a follow-up per ADR-0007. - **JaCoCo gate is still a no-op:** `jacoco:check` skips because no `prepare-agent` execution is bound, so there is no `jacoco.exec`. This predates the PR and is left alone to keep scope. - **Reconcile cutoff:** the cutoff and `indexedAt` values come from application clocks, so nodes are assumed to keep their clocks in sync. - **Update cost:** each update now runs one extra primary-key query (the same pattern `UpdateAction` uses to pre-fetch old records). - Not done here (outside B1): scheduling basepull/reconcile, permission filtering on search results, and replacing the full reindex's wipe-then-rebuild. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
KofTwentyTwo commented 2026-09-26 01:04:11 +00:00 (Migrated from github.com)

CI note: test_only failed before Maven ran. qqq-orb's install_java step (apt-get install on ubuntu-2204:current) hung on the interactive needrestart "Daemons using outdated libraries" dialog and hit the 10-minute no-output timeout (job 43, step "Install Java 21 and build tools").

This is not specific to this PR: the B2/B3 re-pin PRs (qbit-user-role-permissions#5, qbit-customizable-table-views#7) fail the same step the same way, and qqq-orb v0.6.5 has the same install_java.sh. It needs an orb fix, for example DEBIAN_FRONTEND=noninteractive plus NEEDRESTART_MODE=a on the apt calls. The local gates in the description (qqq 4.0.0 and 4.1.0-SNAPSHOT, both with the OpenSearch ITs) pass.

CI note: `test_only` failed before Maven ran. qqq-orb's `install_java` step (`apt-get install` on `ubuntu-2204:current`) hung on the interactive needrestart "Daemons using outdated libraries" dialog and hit the 10-minute no-output timeout (job 43, step "Install Java 21 and build tools"). This is not specific to this PR: the B2/B3 re-pin PRs (qbit-user-role-permissions#5, qbit-customizable-table-views#7) fail the same step the same way, and qqq-orb v0.6.5 has the same `install_java.sh`. It needs an orb fix, for example `DEBIAN_FRONTEND=noninteractive` plus `NEEDRESTART_MODE=a` on the apt calls. The local gates in the description (qqq 4.0.0 and 4.1.0-SNAPSHOT, both with the OpenSearch ITs) pass.
KofTwentyTwo commented 2026-09-26 01:10:50 +00:00 (Migrated from github.com)

Review, round 1 (plan task B1)

Verdict: changes requested. There are two blocking items. The code looks right, and both local builds are green.

Blocking

  1. The qqq-snapshot profile from plan B1 step 3 is missing (pom.xml:226). The plan asks for an opt-in profile qqq-snapshot that imports qqq-bom-pom:${qqq.snapshot.version} (default 4.1.0-SNAPSHOT) ahead of the parent's BOM and adds the Central snapshots repository, and says -Pqqq-snapshot must build and test green. This PR has qqq-version-override, which only turns on when -Dqqq.version is set. On this branch, mvn -Pqqq-snapshot validate prints The requested profile "qqq-snapshot" could not be activated because it does not exist and then resolves qqq-backend-core 4.0.0. So anything that runs the plan's command gets a green build on the wrong qqq line. The plan changed at 19:31, after the re-pin commit at 19:27, which probably explains the mismatch. To fix: rename the profile to qqq-snapshot, add a qqq.snapshot.version property defaulting to 4.1.0-SNAPSHOT, add the snapshots <repository> (the parent already declares one, but the plan wants the profile to stand on its own), and update README.md:349-353 and CLAUDE.md:17,74.
  2. CI is red, and the ITs have not run on CI yet. Job 43 (qqq-orb/mvn_test_only) timed out in "Install Java 21 and build tools". apt stopped at an interactive needrestart "Daemons using outdated libraries" dialog and hit Too long with no output (exceeded 10m0s). No Maven step ran. This is a machine-image or orb problem, not this code, but the check has to go green before merge, and B1 step 4 (ITs fail rather than skip on CI) is only proven once CircleCI actually runs the 12 ITs. Rerun the workflow. If it hangs again, qqq-orb needs NEEDRESTART_MODE=a / DEBIAN_FRONTEND=noninteractive in that step. The last develop build passed in July on the same orb 0.5.2.

Verified

  • In a fresh clone at cf3bbcb, mvn -B clean verify passes: 233 unit tests, 12 ITs, 0 skipped.
  • mvn -B -U -Dmaven.repo.local=<isolated> -Dqqq.version=4.1.0-SNAPSHOT clean verify passes: 233 unit tests and 12 ITs, resolving qqq-backend-core-4.1.0-20260926.003607-7 from Central snapshots.
  • Every other B1 item is done: the parent is 2.0.0 and the child's qqq-bom-pom import is gone, there are no explicit qqq module versions left, Java 21 is set, LICENSE and headers are untouched, the new files copy the neighboring Apache header, root com/ and .claude/settings.local.json are removed and gitignored, and main has no commits that develop lacks.
  • The update re-read with updateInput.getTransaction() and a PK IN query is the same approach qqq 4.0.0 UpdateAction uses to pre-fetch old records. That action does not own the transaction, so the query sees the uncommitted update as intended.
  • A bulk delete of a missing document comes back with no item error and counts as success, and the IT covers it. Keyset paging has a guard for a null key. The purge is scoped by the sourceTable keyword and runs only when there were no indexing errors.
  • No secrets. Admin123! is the existing throwaway Testcontainers password, and the security plugin is disabled.

Non-blocking

  • Purge race (ReconcileIndexStep.java:142,192). Say a real-time index write builds its document before reconcileStart (so its indexedAt is older) but reaches OpenSearch after reconcile has written the same record. It overwrites the fresh document with the older indexedAt, and the purge then deletes a live record's document. Clock skew between nodes has the same effect. The window is small, and the next reconcile repairs it. Moving the cutoff back by a small configurable margin would close it; orphans inside the margin would go on the next run.
  • Purge failures are not checked (QuickSearchOpenSearchClient.java:420-439). DeleteByQueryResponse.failures() and timedOut() are ignored, so a partial purge still reports COMPLETED. Throwing, or counting these as errors, would match how bulk failures are now handled.
  • Delete-by-query runs synchronously. On a large table it could run past the client's socket timeout. waitForCompletion(false) plus polling the task is worth doing later.
  • testReconcile_pagesByPrimaryKey_eachRecordOnce would also pass with offset paging. A variant that deletes a row between batches would lock in the keyset behavior.
  • README.md:361-362 still says the JaCoCo coverage rules are enforced, but jacoco:check skips because prepare-agent is not bound (this predates the PR, as you noted). Fix the wording now, or bind prepare-agent in a follow-up.
  • When there are indexing errors, reconcile returns normally and only the run row says FAILED, the same as FullReindex. It might help to show that in the process output.
  • Real-time indexing still happens before the caller's transaction commits, so a rollback leaves the index ahead of the source. This predates the PR, and reconcile repairs it.

The red-first evidence for the drift fix and the real-OpenSearch drift ITs are solid.

## Review, round 1 (plan task B1) **Verdict: changes requested.** There are two blocking items. The code looks right, and both local builds are green. ### Blocking 1. **The `qqq-snapshot` profile from plan B1 step 3 is missing** (`pom.xml:226`). The plan asks for an opt-in profile `qqq-snapshot` that imports `qqq-bom-pom:${qqq.snapshot.version}` (default `4.1.0-SNAPSHOT`) ahead of the parent's BOM and adds the Central snapshots repository, and says `-Pqqq-snapshot` must build and test green. This PR has `qqq-version-override`, which only turns on when `-Dqqq.version` is set. On this branch, `mvn -Pqqq-snapshot validate` prints `The requested profile "qqq-snapshot" could not be activated because it does not exist` and then resolves `qqq-backend-core` 4.0.0. So anything that runs the plan's command gets a green build on the wrong qqq line. The plan changed at 19:31, after the re-pin commit at 19:27, which probably explains the mismatch. To fix: rename the profile to `qqq-snapshot`, add a `qqq.snapshot.version` property defaulting to `4.1.0-SNAPSHOT`, add the snapshots `<repository>` (the parent already declares one, but the plan wants the profile to stand on its own), and update `README.md:349-353` and `CLAUDE.md:17,74`. 2. **CI is red, and the ITs have not run on CI yet.** Job 43 (`qqq-orb/mvn_test_only`) timed out in "Install Java 21 and build tools". apt stopped at an interactive `needrestart` "Daemons using outdated libraries" dialog and hit `Too long with no output (exceeded 10m0s)`. No Maven step ran. This is a machine-image or orb problem, not this code, but the check has to go green before merge, and B1 step 4 (ITs fail rather than skip on CI) is only proven once CircleCI actually runs the 12 ITs. Rerun the workflow. If it hangs again, `qqq-orb` needs `NEEDRESTART_MODE=a` / `DEBIAN_FRONTEND=noninteractive` in that step. The last develop build passed in July on the same orb 0.5.2. ### Verified - In a fresh clone at `cf3bbcb`, `mvn -B clean verify` passes: 233 unit tests, 12 ITs, 0 skipped. - `mvn -B -U -Dmaven.repo.local=<isolated> -Dqqq.version=4.1.0-SNAPSHOT clean verify` passes: 233 unit tests and 12 ITs, resolving `qqq-backend-core-4.1.0-20260926.003607-7` from Central snapshots. - Every other B1 item is done: the parent is 2.0.0 and the child's `qqq-bom-pom` import is gone, there are no explicit qqq module versions left, Java 21 is set, LICENSE and headers are untouched, the new files copy the neighboring Apache header, root `com/` and `.claude/settings.local.json` are removed and gitignored, and main has no commits that develop lacks. - The update re-read with `updateInput.getTransaction()` and a PK `IN` query is the same approach qqq 4.0.0 `UpdateAction` uses to pre-fetch old records. That action does not own the transaction, so the query sees the uncommitted update as intended. - A bulk delete of a missing document comes back with no item error and counts as success, and the IT covers it. Keyset paging has a guard for a null key. The purge is scoped by the `sourceTable` keyword and runs only when there were no indexing errors. - No secrets. `Admin123!` is the existing throwaway Testcontainers password, and the security plugin is disabled. ### Non-blocking - **Purge race** (`ReconcileIndexStep.java:142,192`). Say a real-time index write builds its document before `reconcileStart` (so its `indexedAt` is older) but reaches OpenSearch after reconcile has written the same record. It overwrites the fresh document with the older `indexedAt`, and the purge then deletes a live record's document. Clock skew between nodes has the same effect. The window is small, and the next reconcile repairs it. Moving the cutoff back by a small configurable margin would close it; orphans inside the margin would go on the next run. - **Purge failures are not checked** (`QuickSearchOpenSearchClient.java:420-439`). `DeleteByQueryResponse.failures()` and `timedOut()` are ignored, so a partial purge still reports COMPLETED. Throwing, or counting these as errors, would match how bulk failures are now handled. - Delete-by-query runs synchronously. On a large table it could run past the client's socket timeout. `waitForCompletion(false)` plus polling the task is worth doing later. - `testReconcile_pagesByPrimaryKey_eachRecordOnce` would also pass with offset paging. A variant that deletes a row between batches would lock in the keyset behavior. - `README.md:361-362` still says the JaCoCo coverage rules are enforced, but `jacoco:check` skips because `prepare-agent` is not bound (this predates the PR, as you noted). Fix the wording now, or bind `prepare-agent` in a follow-up. - When there are indexing errors, reconcile returns normally and only the run row says FAILED, the same as FullReindex. It might help to show that in the process output. - Real-time indexing still happens before the caller's transaction commits, so a rollback leaves the index ahead of the source. This predates the PR, and reconcile repairs it. The red-first evidence for the drift fix and the real-OpenSearch drift ITs are solid.
KofTwentyTwo commented 2026-09-26 01:29:41 +00:00 (Migrated from github.com)

Both blocking findings are fixed in 4e1fe03 and 7cd94ae.

1. qqq-snapshot profile (plan B1 step 3), commit 4e1fe03

  • pom.xml: renamed the profile to qqq-snapshot. It is now opt-in only (it has no property activation). It sets qqq.snapshot.version to 4.1.0-SNAPSHOT by default, imports qqq-bom-pom:${qqq.snapshot.version} ahead of the parent's BOM, and adds the Central snapshots repository (https://central.sonatype.com/repository/maven-snapshots/). This matches the profile in the B2 and B3 PRs.
  • Checked resolution: -Pqqq-snapshot resolves qqq-backend-core and qqq-backend-module-rdbms to 4.1.0-SNAPSHOT. The default build still resolves 4.0.0 from qbit-build-parent.
  • Updated README.md (Testing) and CLAUDE.md (Build Commands, Dependencies) to describe -Pqqq-snapshot and -Dqqq.snapshot.version. They no longer mention -Dqqq.version or qqq-version-override.

2. CircleCI, commit 7cd94ae

  • The push of 4e1fe03 started a fresh pipeline. Its job 44 hung the same way as job 43: the orb's install_java step stopped at needrestart's "Daemons using outdated libraries" dialog on ubuntu-2204:current and hit the 10-minute no-output timeout.
  • I added the configure-package-install pre-step already used by qbit-bom and qbit-template-extension. It writes $nrconf{restart} = "l"; to /etc/needrestart/conf.d/99-qqq-ci.conf and runs before every orb job, including the publish jobs, which use the same install step. The config passes circleci config validate.
  • Job 45 passed. Unit tests: 233 run, 0 failed, 0 skipped. Failsafe: 12 run, 0 failed, 0 skipped. So B1 step 4 is now proven on CI: the Docker integration tests actually run there.
  • Follow-up outside this PR: every repo on qqq-orb 0.5.x can hit this. A lasting fix in qqq-orb's install_java.sh would run apt with NEEDRESTART_MODE=a DEBIAN_FRONTEND=noninteractive, passed through sudo. After that ships, this pre-step can be removed.

Local gate (no mvnw in this repo; Docker running; CI=true)

  • mvn -B verify: BUILD SUCCESS. 233 unit tests and 12 integration tests, 0 failed, 0 skipped. JaCoCo check passed.
  • mvn -B verify -Pqqq-snapshot (qqq 4.1.0-SNAPSHOT): BUILD SUCCESS with the same counts.

Not merging.

Both blocking findings are fixed in 4e1fe03 and 7cd94ae. **1. `qqq-snapshot` profile (plan B1 step 3), commit 4e1fe03** - `pom.xml`: renamed the profile to `qqq-snapshot`. It is now opt-in only (it has no property activation). It sets `qqq.snapshot.version` to `4.1.0-SNAPSHOT` by default, imports `qqq-bom-pom:${qqq.snapshot.version}` ahead of the parent's BOM, and adds the Central snapshots repository (`https://central.sonatype.com/repository/maven-snapshots/`). This matches the profile in the B2 and B3 PRs. - Checked resolution: `-Pqqq-snapshot` resolves `qqq-backend-core` and `qqq-backend-module-rdbms` to 4.1.0-SNAPSHOT. The default build still resolves 4.0.0 from `qbit-build-parent`. - Updated `README.md` (Testing) and `CLAUDE.md` (Build Commands, Dependencies) to describe `-Pqqq-snapshot` and `-Dqqq.snapshot.version`. They no longer mention `-Dqqq.version` or `qqq-version-override`. **2. CircleCI, commit 7cd94ae** - The push of 4e1fe03 started a fresh pipeline. Its job 44 hung the same way as job 43: the orb's `install_java` step stopped at needrestart's "Daemons using outdated libraries" dialog on `ubuntu-2204:current` and hit the 10-minute no-output timeout. - I added the `configure-package-install` pre-step already used by `qbit-bom` and `qbit-template-extension`. It writes `$nrconf{restart} = "l";` to `/etc/needrestart/conf.d/99-qqq-ci.conf` and runs before every orb job, including the publish jobs, which use the same install step. The config passes `circleci config validate`. - Job 45 passed. Unit tests: 233 run, 0 failed, 0 skipped. Failsafe: 12 run, 0 failed, 0 skipped. So B1 step 4 is now proven on CI: the Docker integration tests actually run there. - Follow-up outside this PR: every repo on qqq-orb 0.5.x can hit this. A lasting fix in qqq-orb's `install_java.sh` would run apt with `NEEDRESTART_MODE=a DEBIAN_FRONTEND=noninteractive`, passed through `sudo`. After that ships, this pre-step can be removed. **Local gate** (no mvnw in this repo; Docker running; `CI=true`) - `mvn -B verify`: BUILD SUCCESS. 233 unit tests and 12 integration tests, 0 failed, 0 skipped. JaCoCo check passed. - `mvn -B verify -Pqqq-snapshot` (qqq 4.1.0-SNAPSHOT): BUILD SUCCESS with the same counts. Not merging.
KofTwentyTwo commented 2026-09-26 01:35:16 +00:00 (Migrated from github.com)

Review, round 2 (plan task B1)

Verdict: approve. Nothing is blocking. Both round-1 blockers are fixed at 7cd94ae. Recommendation only; merging is a human decision.

Routing: local--review-pr sized this as Deep (1689/2397 lines across 29 files; risk signals: .circleci/config.yml and dependency changes in pom.xml) and sent it to the pr-review-toolkit aspects (code, tests, error handling, docs, build/CI). Round 2 re-checked the delta since cf3bbcb (4e1fe03, 7cd94ae) and went back over B1 as a whole.

Round-1 blockers

  1. qqq-snapshot profile (B1 step 3): fixed.
    • pom.xml:226-253 matches the plan: an opt-in qqq-snapshot profile (no activation), qqq.snapshot.version defaulting to 4.1.0-SNAPSHOT, an import of qqq-bom-pom:${qqq.snapshot.version}, and the Central snapshots repository.
    • Effective POM without the profile: qqq-backend-core 4.0.0. With -Pqqq-snapshot: qqq-backend-core 4.1.0-SNAPSHOT, with httpclient5 5.6.3 and httpcore5 5.4.3 in both. The profile's import replaces the parent's 4.0.0 import instead of sitting behind it.
    • README.md:349-355 and CLAUDE.md:17,74 describe the new flag. No qqq-version-override or -Dqqq.version references remain in the repo.
  2. CI green, with the ITs running there: fixed.
    • CircleCI job 45 on 7cd94ae passed. Its "Run Maven Verify" log shows failsafe Tests run: 12, Failures: 0, Errors: 0, Skipped: 0 and BUILD SUCCESS, so B1 step 4 now holds on CI.
    • The configure-package-install pre-step is safe on every workflow it was added to. In qqq-orb 0.5.2, both mvn_test_only and mvn_publish use the java machine executor (ubuntu-2204:current), so sudo and needrestart are there. It matches the step in qbit-template-extension.

Verified locally (fresh clones at 7cd94ae, Java 21.0.12, Maven 3.9.16, Docker 29.8.0, CI=true, no mvn install)

Command Result
mvn -B clean verify BUILD SUCCESS: 233 unit tests and 12 ITs, 0 failures, 0 skipped
mvn -B -U -Dmaven.repo.local=<isolated, no 4.1 snapshots> -Pqqq-snapshot clean verify BUILD SUCCESS: 233 unit tests and 12 ITs, 0 failures, 0 skipped; qqq-bom-pom 4.1.0-20260926.003607-9 and qqq-backend-core 4.1.0-20260926.003607-7 resolved from Central snapshots through the profile's sonatype-snapshots repository

B1 checklist at head:

  • main has no commits that develop lacks.
  • The parent is qbit-build-parent:2.0.0, and the only qqq-bom-pom import left is the one in the profile.
  • No qqq module <version>s remain, and Java is 21.
  • LICENSE files and existing headers are untouched; the 5 new source files copy the neighboring Apache-2.0 header.
  • Root com/ and .claude/settings.local.json are removed, and .gitignore:21 covers the settings file.
  • The drift fix and the reconcile process are unchanged since round 1, and their unit tests and real-OpenSearch ITs pass.
  • No secrets. Admin123! is the throwaway Testcontainers password with the security plugin disabled, and secretPassword in the README is an old placeholder.
  • No checkstyle plugin is bound in this repo or its parent, so there is no style gate to fail.

Non-blocking

  1. The PR description is out of date. It still describes the qqq-version-override profile and -Dqqq.version: the "Re-pin (build)" bullet, the 4.1.0-SNAPSHOT row in the Testing table, and the effective-pom row. It also leaves the CircleCI pre-step out of Changes. Update it before merge so the merge message matches the code.
  2. The JaCoCo gate still does nothing. The round-2 note says "JaCoCo check passed", but both locally and in CI job 45 jacoco:check logs Skipping JaCoCo execution due to missing execution data file. README.md:362-363 still says "(JaCoCo enforced)". This predates the PR (the child's JaCoCo config is unchanged, and neither parent 1.6.0 nor 2.0.0 binds prepare-agent), so it belongs in a follow-up. Either bind prepare-agent or reword the README.
  3. The needrestart pre-step is a per-repo workaround. The lasting fix belongs in qqq-orb's install_java.sh (NEEDRESTART_MODE=a, DEBIAN_FRONTEND=noninteractive passed through sudo), as you noted. Remove the pre-step once that ships.
  4. Round-1 non-blocking items are still open. Worth one follow-up issue:
    • the purge race / cutoff margin (ReconcileIndexStep.java:142,192);
    • DeleteByQueryResponse.failures() and timedOut() are ignored (QuickSearchOpenSearchClient.java:420-445);
    • delete-by-query runs synchronously on large tables;
    • no keyset-paging test that deletes a row between batches;
    • a FAILED reconcile run shows only on the run row, not in the process output.

🤖 Generated with Claude Code

## Review, round 2 (plan task B1) **Verdict: approve.** Nothing is blocking. Both round-1 blockers are fixed at `7cd94ae`. Recommendation only; merging is a human decision. Routing: `local--review-pr` sized this as Deep (1689/2397 lines across 29 files; risk signals: `.circleci/config.yml` and dependency changes in `pom.xml`) and sent it to the `pr-review-toolkit` aspects (code, tests, error handling, docs, build/CI). Round 2 re-checked the delta since `cf3bbcb` (`4e1fe03`, `7cd94ae`) and went back over B1 as a whole. ### Round-1 blockers 1. **`qqq-snapshot` profile (B1 step 3): fixed.** - `pom.xml:226-253` matches the plan: an opt-in `qqq-snapshot` profile (no activation), `qqq.snapshot.version` defaulting to `4.1.0-SNAPSHOT`, an import of `qqq-bom-pom:${qqq.snapshot.version}`, and the Central snapshots repository. - Effective POM without the profile: `qqq-backend-core` 4.0.0. With `-Pqqq-snapshot`: `qqq-backend-core` 4.1.0-SNAPSHOT, with httpclient5 5.6.3 and httpcore5 5.4.3 in both. The profile's import replaces the parent's 4.0.0 import instead of sitting behind it. - `README.md:349-355` and `CLAUDE.md:17,74` describe the new flag. No `qqq-version-override` or `-Dqqq.version` references remain in the repo. 2. **CI green, with the ITs running there: fixed.** - CircleCI job 45 on `7cd94ae` passed. Its "Run Maven Verify" log shows failsafe `Tests run: 12, Failures: 0, Errors: 0, Skipped: 0` and BUILD SUCCESS, so B1 step 4 now holds on CI. - The `configure-package-install` pre-step is safe on every workflow it was added to. In qqq-orb 0.5.2, both `mvn_test_only` and `mvn_publish` use the `java` machine executor (`ubuntu-2204:current`), so `sudo` and needrestart are there. It matches the step in `qbit-template-extension`. ### Verified locally (fresh clones at `7cd94ae`, Java 21.0.12, Maven 3.9.16, Docker 29.8.0, `CI=true`, no `mvn install`) | Command | Result | |---|---| | `mvn -B clean verify` | BUILD SUCCESS: 233 unit tests and 12 ITs, 0 failures, 0 skipped | | `mvn -B -U -Dmaven.repo.local=<isolated, no 4.1 snapshots> -Pqqq-snapshot clean verify` | BUILD SUCCESS: 233 unit tests and 12 ITs, 0 failures, 0 skipped; `qqq-bom-pom` 4.1.0-20260926.003607-9 and `qqq-backend-core` 4.1.0-20260926.003607-7 resolved from Central snapshots through the profile's `sonatype-snapshots` repository | B1 checklist at head: - `main` has no commits that `develop` lacks. - The parent is `qbit-build-parent:2.0.0`, and the only `qqq-bom-pom` import left is the one in the profile. - No qqq module `<version>`s remain, and Java is 21. - LICENSE files and existing headers are untouched; the 5 new source files copy the neighboring Apache-2.0 header. - Root `com/` and `.claude/settings.local.json` are removed, and `.gitignore:21` covers the settings file. - The drift fix and the reconcile process are unchanged since round 1, and their unit tests and real-OpenSearch ITs pass. - No secrets. `Admin123!` is the throwaway Testcontainers password with the security plugin disabled, and `secretPassword` in the README is an old placeholder. - No checkstyle plugin is bound in this repo or its parent, so there is no style gate to fail. ### Non-blocking 1. **The PR description is out of date.** It still describes the `qqq-version-override` profile and `-Dqqq.version`: the "Re-pin (build)" bullet, the 4.1.0-SNAPSHOT row in the Testing table, and the effective-pom row. It also leaves the CircleCI pre-step out of Changes. Update it before merge so the merge message matches the code. 2. **The JaCoCo gate still does nothing.** The round-2 note says "JaCoCo check passed", but both locally and in CI job 45 `jacoco:check` logs `Skipping JaCoCo execution due to missing execution data file`. `README.md:362-363` still says "(JaCoCo enforced)". This predates the PR (the child's JaCoCo config is unchanged, and neither parent 1.6.0 nor 2.0.0 binds `prepare-agent`), so it belongs in a follow-up. Either bind `prepare-agent` or reword the README. 3. **The needrestart pre-step is a per-repo workaround.** The lasting fix belongs in qqq-orb's `install_java.sh` (`NEEDRESTART_MODE=a`, `DEBIAN_FRONTEND=noninteractive` passed through `sudo`), as you noted. Remove the pre-step once that ships. 4. **Round-1 non-blocking items are still open.** Worth one follow-up issue: - the purge race / cutoff margin (`ReconcileIndexStep.java:142,192`); - `DeleteByQueryResponse.failures()` and `timedOut()` are ignored (`QuickSearchOpenSearchClient.java:420-445`); - delete-by-query runs synchronously on large tables; - no keyset-paging test that deletes a row between batches; - a FAILED reconcile run shows only on the run row, not in the process output. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Sign in to join this conversation.