fix: keep the live index after an alias swap, atomic legacy upgrade, exact paging under record locks

From the PR #10 review: never delete the new physical index once the alias points at it, upgrade a legacy concrete index in one aliases request, base hasMore on raw hits so filtered pages stay reachable, and deduplicate requested table names.
This commit is contained in:
james.maes committed 2026-10-05 07:13:31 -05:00
1 parent e88a459d85
commit 45bd1fce57
4 files changed
+45 -20

No files matched your search

+10 -2
View File
@@ -2,16 +2,24 @@
## Before tagging 1.0.0
- [ ] Review and merge `feature/GH-1-0-release-readiness` (8 commits; nothing pushed). Decide whether to keep `AGENTS.md`.
- [x] PR #10 (`feature/GH-1-0-release-readiness`) opened and reviewed 2026-10-05; `AGENTS.md` kept.
- [ ] Re-pin to `qbit-build-parent` 2.1.0 when qbit-bom publishes it, and remove the `activeByDefault` on the `qqq-snapshot` profile (ADR-0007).
- [ ] qqq-all follow-up PR: remove the manual `instance.addQBit(...)` for quick-search in `DemoQBits`, pass `withSchedulerName("demoScheduler")`, set `withStartupMode(QuickSearchStartupMode.DEGRADED)` for the full profile test, pin 1.0.0.
- [ ] Voyage: pin 1.0.0, set `schedulerName`, switch to `opensearchUrl` and `${env.}` credentials.
- [ ] CI: add a second test job with `-Dopensearch.test.image=opensearchproject/opensearch:3.9.0` (needs a qqq-orb parameter for extra Maven args, or a plain job).
- [ ] Open the qqq issue proposing `RecordSearchProviderInterface` (D1); quick-search adapter lands in 1.1.
- [ ] Create the GitHub tracking issue for the 1.0 initiative; write its number into the PRD frontmatter.
- [ ] Review and merge PR #8; close Dependabot PR #1 as superseded.
- [ ] Merge PR #8 (reviewed 2026-10-05, approve); Dependabot PR #1 closed as superseded.
- [ ] Upstream: tell qbit-bom that the published parent POM has `<version>${revision}</version>`, which Maven 3.10 rejects (this repo works around it with the wrapper on 3.9.11).
## Before 1.0.0 GA (from the PR #10 review, 2026-10-05)
- [ ] Listener: if the in-transaction re-read in `fetchCurrentRecords` throws, record a `quickSearchFailedEvent` row from the event's primary keys instead of dropping the update (`QuickSearchRecordChangeListener`).
- [ ] Full reindex: deletes that arrive during a rebuild are not replayed into the new index; capture them (failed-event or a delete log) and apply after the swap.
- [ ] Transport: warn when `tls.hostnameVerification=false`; treat an empty `${env.X}` value as missing and name the field; reject userinfo in `opensearchUrl`.
- [ ] Search: with record locks on, `offset` counts raw hits, so pages can overlap; per-table mode ignores `offset`; cap `fetchSize` at the 10,000 window.
- [ ] `safeCount` in `BasepullIndexStep` swallows exceptions silently; log at warn.
## Later (1.x)
- [ ] Health widget for the admin app (data exists: `isHealthy`, `documentCount`, `lastRunStatus`).
@@ -22,6 +22,7 @@ import java.util.ArrayList;
import java.util.Collections;
import java.util.HashSet;
import java.util.LinkedHashMap;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
@@ -145,13 +146,11 @@ public class QuickSearchAction
SearchResponse<OpenSearchDocument> response = runSearch(client, normalizedTerm, allowedTables, fetchSize, offset, tableConfigs);
List<QuickSearchResult> results = toResults(response);
boolean filtered = false;
int rawHits = results.size();
if(applyLocks)
{
int before = results.size();
results = filterByRecordAccess(results, tableConfigs);
filtered = results.size() != before;
results = filterByRecordAccess(results, tableConfigs);
}
boolean truncated = results.size() > limit;
@@ -162,7 +161,7 @@ public class QuickSearchAction
long totalHits = response.hits().total() != null ? response.hits().total().value() : results.size();
boolean lowerBound = response.hits().total() != null && response.hits().total().relation() == TotalHitsRelation.Gte;
boolean hasMore = truncated || (offset + results.size()) < totalHits;
boolean hasMore = truncated || (offset + rawHits) < totalHits;
if(applyLocks)
{
@@ -171,8 +170,8 @@ public class QuickSearchAction
// not see; report only what was verified accessible //
///////////////////////////////////////////////////////////////////////
totalHits = offset + results.size() + (truncated ? 1 : 0);
lowerBound = truncated || (!filtered && hasMore);
hasMore = truncated || (!filtered && hasMore);
lowerBound = truncated || hasMore;
hasMore = truncated || hasMore;
}
return (new QuickSearchOutput()
@@ -256,7 +255,7 @@ public class QuickSearchAction
boolean checkPermissions = QContext.getQInstance() != null && QContext.getQSession() != null;
List<String> allowed = new ArrayList<>();
for(String tableName : requested)
for(String tableName : new LinkedHashSet<>(requested))
{
if(!configured.contains(tableName))
{
@@ -279,8 +279,13 @@ public class QuickSearchOpenSearchClient implements Closeable
if(previous.isEmpty() && client.indices().exists(ExistsRequest.of(r -> r.index(indexName))).value())
{
client.indices().delete(d -> d.index(indexName));
client.indices().updateAliases(u -> u.actions(a -> a.add(ad -> ad.index(newPhysicalIndex).alias(indexName))));
///////////////////////////////////////////////////////////////////////////
// one request: an alias cannot share the legacy index's name, and two //
// calls would leave a window with neither index nor alias //
///////////////////////////////////////////////////////////////////////////
client.indices().updateAliases(u -> u
.actions(a -> a.removeIndex(ri -> ri.index(indexName)))
.actions(a -> a.add(ad -> ad.index(newPhysicalIndex).alias(indexName))));
}
else
{
@@ -297,7 +302,14 @@ public class QuickSearchOpenSearchClient implements Closeable
{
if(!old.equals(newPhysicalIndex))
{
client.indices().delete(d -> d.index(old));
try
{
client.indices().delete(d -> d.index(old));
}
catch(OpenSearchException | java.io.IOException cleanup)
{
LOG.warn("Alias swapped but the previous physical index could not be deleted", cleanup, logPair("physicalIndex", old));
}
}
}
}
@@ -116,6 +116,8 @@ public class FullReindexStep extends AbstractIndexingStep
Map<String, Instant> startTimes = new LinkedHashMap<>();
Integer indexed = 0;
boolean swapped = false;
try
{
for(QuickSearchableTableConfig tableConfig : tables)
@@ -137,6 +139,7 @@ public class FullReindexStep extends AbstractIndexingStep
client.refreshIndex(physicalIndex);
client.swapAliasTo(physicalIndex);
swapped = true;
for(QuickSearchableTableConfig tableConfig : tables)
{
@@ -160,7 +163,7 @@ public class FullReindexStep extends AbstractIndexingStep
}
catch(Exception e)
{
LOG.warn("Full reindex failed; the previous index stays in service", e, logPair("physicalIndex", physicalIndex));
LOG.warn(swapped ? "Full reindex failed after the alias swap; the new index stays in service" : "Full reindex failed; the previous index stays in service", e, logPair("physicalIndex", physicalIndex));
for(Map.Entry<QuickSearchableTableConfig, QuickSearchIndexRun> entry : runs.entrySet())
{
@@ -172,13 +175,16 @@ public class FullReindexStep extends AbstractIndexingStep
e.getMessage());
}
try
if(!swapped)
{
client.deletePhysicalIndex(physicalIndex);
}
catch(Exception cleanup)
{
LOG.warn("Could not delete the incomplete physical index", cleanup, logPair("physicalIndex", physicalIndex));
try
{
client.deletePhysicalIndex(physicalIndex);
}
catch(Exception cleanup)
{
LOG.warn("Could not delete the incomplete physical index", cleanup, logPair("physicalIndex", physicalIndex));
}
}
if(e instanceof QException qException)