You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@polaris.apache.org by "vigneshio (via GitHub)" <gi...@apache.org> on 2026/10/05 09:10:55 UTC
[PR] fix(nosql): retain spilled IndexStripeObj segments during maintenance [polaris]
vigneshio opened a new pull request, #5701:
URL: https://github.com/apache/polaris/pull/5701
NoSQL maintenance could delete spilled `IndexStripeObj` (`ix`) segments that catalog indexes still needed.
Once an index exceeds `maxEmbeddedIndexSize` (default `32k`), entries live in separate stripe objs. `CatalogRetainedIdentifier` walked indexes like `nameToObjRef` / ACLs / policy mappings during identify, but only called `indexRetain` on `stableIdToName` / `locations` / `changes`. `refRetainIndexToSingleObj` retained the indexed entity objs, not the stripe segments that hold the keys. After purge, the next index read fails with `references a reference index, which does not exist`.
## Fix
- `RetainedCollector.refRetainIndexToSingleObj` now `indexRetain`s the `IndexContainer` it walks (covers catalog-state `nameToObjRef`, container `nameToObjRef`, ACL indexes, realm index, etc.)
- `CatalogRetainedIdentifier` also retains stripes for manually walked indexes (`CatalogsObj.nameToObjRef`, policy mappings) and `PrincipalsObj.byClientId`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(nosql): retain spilled IndexStripeObj segments during maintenance [polaris]
Posted by "snazy (via GitHub)" <gi...@apache.org>.
snazy merged PR #5701:
URL: https://github.com/apache/polaris/pull/5701
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(nosql): retain spilled IndexStripeObj segments during maintenance [polaris]
Posted by "vigneshio (via GitHub)" <gi...@apache.org>.
vigneshio commented on code in PR #5701:
URL: https://github.com/apache/polaris/pull/5701#discussion_r4182926227
##########
persistence/nosql/persistence/maintenance/spi/src/main/java/org/apache/polaris/persistence/nosql/maintenance/spi/RetainedCollector.java:
##########
@@ -212,10 +212,12 @@ default <O extends BaseCommitObj> void refRetainIndexToSingleObj(
// inspecting already seen index segments (as a performance optimization.), but that
// requires some changes to the index APIs.
- for (var entry :
- indexToObjIdFromRetainedObj
- .apply(obj)
- .indexForRead(realmPersistence(), OBJ_REF_SERIALIZER)) {
+ // Walking an index via indexForRead does not retain IndexStripeObj segments; those are
+ // separate objs and must be kept explicitly or maintenance can purge them.
+ var indexContainer = indexToObjIdFromRetainedObj.apply(obj);
+ indexRetain(indexContainer);
+
+ for (var entry : indexContainer.indexForRead(realmPersistence(), OBJ_REF_SERIALIZER)) {
Review Comment:
Good call - yeah , that fits better in `buildReadIndex` since that’s what `indexForRead` actually uses, similar to `fetch → retainObject`.
I’ve pushed an update that moves it there and removes the extra retains from `refRetainIndexToSingleObj`, the policy mappings, and `catalogs.nameToObjRef`.
I’ve kept the `byClientId` index retain since `identify` never goes through `indexForRead`, so `buildReadIndex` won’t cover that case.
PTAL..
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(nosql): retain spilled IndexStripeObj segments during maintenance [polaris]
Posted by "snazy (via GitHub)" <gi...@apache.org>.
snazy commented on code in PR #5701:
URL: https://github.com/apache/polaris/pull/5701#discussion_r4182711297
##########
persistence/nosql/persistence/maintenance/spi/src/main/java/org/apache/polaris/persistence/nosql/maintenance/spi/RetainedCollector.java:
##########
@@ -212,10 +212,12 @@ default <O extends BaseCommitObj> void refRetainIndexToSingleObj(
// inspecting already seen index segments (as a performance optimization.), but that
// requires some changes to the index APIs.
- for (var entry :
- indexToObjIdFromRetainedObj
- .apply(obj)
- .indexForRead(realmPersistence(), OBJ_REF_SERIALIZER)) {
+ // Walking an index via indexForRead does not retain IndexStripeObj segments; those are
+ // separate objs and must be kept explicitly or maintenance can purge them.
+ var indexContainer = indexToObjIdFromRetainedObj.apply(obj);
+ indexRetain(indexContainer);
+
+ for (var entry : indexContainer.indexForRead(realmPersistence(), OBJ_REF_SERIALIZER)) {
Review Comment:
I think, there's a way to implement this a bit nicer by adding 3 lines to `RetainedCollectorImpl.buildReadIndex()`:
```
@Override
public <V> Index<V> buildReadIndex(
@Nullable IndexContainer<V> indexContainer,
@NonNull IndexValueSerializer<V> indexValueSerializer) {
if (indexContainer != null) {
indexRetain(indexContainer);
}
return persistence.buildReadIndex(indexContainer, indexValueSerializer);
}
```
That would make the change here and the change around line 184 in `CatalogRetainedIdentifier` superfluous.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org