snazy commented on code in PR #3267:
URL: https://github.com/apache/polaris/pull/3267#discussion_r2642342349
##########
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:
Realm-managment defines possible realm-states and allowed transitions
between those.
The intent, in this "purge realm" case, is to later allow a workflow to
first set a realm to `INACTIVE` (aka it's still there, but you cannot use it),
allowing users to revert their decision within a couple of days, pretty much
what people can do in other SaaS offerings. From `INACTIVE` it can transition
to `PURGING`, which means that the management-service takes care of actually
deleting all realm data and eventually delete the realm definition itself.
##########
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:
It is a bit awkward, true. OTOH, `InMemory` is the only backend that doesn't
persist anything, so we cannot use the management-service or even the
admin-tool here.
`PolarisIntegrationTestFixture` can't be used in all tests, as it's only for
Quarkus tests.
This change actually also applies when people try Polaris server w/ the
`InMemory` backend.
--
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]
