dimas-b commented on code in PR #3267:
URL: https://github.com/apache/polaris/pull/3267#discussion_r2641366111
##########
persistence/nosql/persistence/metastore/src/main/java/org/apache/polaris/persistence/nosql/metastore/NoSqlMetaStoreManagerFactory.java:
##########
@@ -198,20 +198,28 @@ BaseResult purgeRealm(String realmId) {
var nextStatus =
switch (existing.status()) {
- case CREATED, LOADING, INITIALIZING, INACTIVE -> PURGING;
+ case CREATED, LOADING, INITIALIZING, INACTIVE, PURGING ->
+ // PURGING is the state telling the maintenance service to
delete the realm data.
+ PURGING;
case ACTIVE -> INACTIVE;
- case PURGING ->
- // TODO this state should really happen during maintenance!!
+ case PURGED ->
+ // The PURGED state is also handled by the maintenance service.
PURGED;
- case PURGED -> PURGED;
};
+ if (nextStatus == existing.status()) {
+ // No status change (PURGING/PURGED), stop here.
+ // Maintenance service will handle the actual deletion of the realm
data.
+ break;
+ }
+
var update =
RealmDefinition.builder().from(existing).status(nextStatus).build();
var updated = realmManagement.update(existing, update);
- if (updated.status() == PURGED) {
- realmManagement.delete(updated);
+ if (updated.status() == PURGING || updated.status() == PURGED) {
Review Comment:
Do we really need the complex state transition sequence (via `INACTIVE`)
with a loop now? Why not transition to the final state in one step?
##########
persistence/nosql/realms/store-nosql/src/main/java/org/apache/polaris/persistence/nosql/realms/store/RealmStoreImpl.java:
##########
@@ -206,7 +211,18 @@ public RealmDefinition update(
return state.commitResult(obj, newRealms, refObj);
});
- return objToDefinition(realmId, realm.orElseThrow());
+ var result = realm.orElseThrow();
+ if (result.status() == PURGING && "InMemory".equals(backend.type())) {
+ // Handle the test/development case when using the in-memory backend:
+ // In this case there is no chance to run the maintenance service. To
remove no longer
+ // necessary data from the Java heap, call the in-memory backend
directly.
+ // This is "nice behavior" when running tests.
+ // The only "cost" is that the state PURGING may never be pushed to
PURGED, but that is
+ // acceptable.
+ backend.deleteRealms(Set.of(realmId));
Review Comment:
This looks awkward... Could we delegate that to `Backend` impl?... or maybe
move the special case to `PolarisIntegrationTestFixture`?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
