You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@polaris.apache.org by sn...@apache.org on 2026/10/11 13:53:05 UTC
(polaris) branch main updated: Node management: ensure that node leases are released on shutdown (#5744)
This is an automated email from the ASF dual-hosted git repository.
snazy pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/polaris.git
The following commit(s) were added to refs/heads/main by this push:
new 25f1b94b54 Node management: ensure that node leases are released on shutdown (#5744)
25f1b94b54 is described below
commit 25f1b94b542d95fef394c56b0cad3818e1037536
Author: Robert Stupp <sn...@snazy.de>
AuthorDate: Sun Oct 11 15:53:00 2026 +0200
Node management: ensure that node leases are released on shutdown (#5744)
CDI guarantees no dependency ordering for `@PreDestroy` annotated functions (non-`@Dependent` case).
This means, that a `@PreDestroy` can be invoked although the `@Inject`ed proxyies refer to a "no longer valid" bean instance.
In practice, it can happen that `NodeManagementImpl.close()` is called _after_ the database client has been closed, leading to node leases not being released.
Such a case does not represent a shutdown failure scenario.
Given that there are 2^10-1 possible active node leases, it is likely not an issue in practice, but still a correctness concern.
Also update the logged messages during a release operation.
This change adds a `@BeforeDestroyed` observer to `NodeManagementImpl`, which is guaranteed to run before a database client is closed.
Also add a `@Disposes` handler for the `Backend` instance. This is effectively a no-op for Quarkus managed database clients, but also a correctness fix.
---
.../nosql/nodeids/impl/NodeManagementImpl.java | 64 ++++++++++++++++------
.../nosql/quarkus/backend/BackendProvider.java | 5 ++
2 files changed, 53 insertions(+), 16 deletions(-)
diff --git a/persistence/nosql/nodes/impl/src/main/java/org/apache/polaris/persistence/nosql/nodeids/impl/NodeManagementImpl.java b/persistence/nosql/nodes/impl/src/main/java/org/apache/polaris/persistence/nosql/nodeids/impl/NodeManagementImpl.java
index 66d40f111f..7b88b2ee8c 100644
--- a/persistence/nosql/nodes/impl/src/main/java/org/apache/polaris/persistence/nosql/nodeids/impl/NodeManagementImpl.java
+++ b/persistence/nosql/nodes/impl/src/main/java/org/apache/polaris/persistence/nosql/nodeids/impl/NodeManagementImpl.java
@@ -26,7 +26,11 @@ import com.google.common.annotations.VisibleForTesting;
import com.google.common.primitives.Ints;
import jakarta.annotation.PostConstruct;
import jakarta.annotation.PreDestroy;
+import jakarta.annotation.Priority;
import jakarta.enterprise.context.ApplicationScoped;
+import jakarta.enterprise.context.BeforeDestroyed;
+import jakarta.enterprise.event.Observes;
+import jakarta.enterprise.event.Reception;
import jakarta.inject.Inject;
import java.net.InterfaceAddress;
import java.net.NetworkInterface;
@@ -333,6 +337,27 @@ class NodeManagementImpl implements NodeManagement {
throw new IllegalStateException("Could not lease any node ID");
}
+ /**
+ * Observe {@link BeforeDestroyed @BeforeDestroyed} to ensure that a persistence backend is still
+ * "alive".
+ *
+ * <p>A {@link PreDestroy @PreDestroy} on {@link #close()} can run <em>after</em> a persistence
+ * backend or its database client has been closed. Running {@link #close()} too late risks that
+ * any {@link NodeLease lease} cannot be released and remains unusable until the lease expires,
+ * see {@link NodeManagementConfig#leaseDuration()}. The {@link Priority priority} value of {@code
+ * 2900} (@link {@link jakarta.interceptor.Interceptor.Priority#LIBRARY_AFTER}{@code - 100} is
+ * meant to make {@link #close()} run quite before any {@link PreDestroy @PreDestroy} runs.
+ *
+ * <p>{@link #close()} retains the {@link PreDestroy @PreDestroy} as an idempotent fallback.
+ */
+ void releaseLeasesBeforeApplicationShutdown(
+ @Observes(notifyObserver = Reception.IF_EXISTS)
+ @Priority(2900)
+ @BeforeDestroyed(ApplicationScoped.class)
+ Object ignored) {
+ close();
+ }
+
@Override
@PreDestroy
public void close() {
@@ -542,22 +567,29 @@ class NodeManagementImpl implements NodeManagement {
return;
}
var id = lp.nodeId;
- LOGGER.info("Releasing lease for node id {}", id);
-
- var now = clock.currentInstant();
- var activeState = lp.nodeState;
- var nodeAsReleased =
- ImmutableNodeState.builder().from(activeState).expirationTimestamp(now).build();
-
- leaseParams = null;
-
- var updated = nodeStore.persist(id, Optional.of(activeState), nodeAsReleased);
- checkState(
- updated != null && updated.equals(nodeAsReleased),
- "State of the node %s has been unexpectedly changed: value of persisted %s != to-persist %s",
- id,
- updated,
- nodeAsReleased);
+ LOGGER.debug("Releasing lease for node id {}...", id);
+
+ try {
+ var now = clock.currentInstant();
+ var activeState = lp.nodeState;
+ var nodeAsReleased =
+ ImmutableNodeState.builder().from(activeState).expirationTimestamp(now).build();
+
+ leaseParams = null;
+
+ var updated = nodeStore.persist(id, Optional.of(activeState), nodeAsReleased);
+ checkState(
+ updated != null && updated.equals(nodeAsReleased),
+ "State of the node %s has been unexpectedly changed: value of persisted %s != to-persist %s",
+ id,
+ updated,
+ nodeAsReleased);
+
+ LOGGER.info("Released lease for node id {}", id);
+ } catch (RuntimeException e) {
+ throw new RuntimeException(
+ String.format("Failed to release lease for node id %d: %s", id, e), e);
+ }
} finally {
lock.unlock();
}
diff --git a/persistence/nosql/persistence/cdi/quarkus/src/main/java/org/apache/polaris/persistence/nosql/quarkus/backend/BackendProvider.java b/persistence/nosql/persistence/cdi/quarkus/src/main/java/org/apache/polaris/persistence/nosql/quarkus/backend/BackendProvider.java
index 52f1a6ae20..f799944d5b 100644
--- a/persistence/nosql/persistence/cdi/quarkus/src/main/java/org/apache/polaris/persistence/nosql/quarkus/backend/BackendProvider.java
+++ b/persistence/nosql/persistence/cdi/quarkus/src/main/java/org/apache/polaris/persistence/nosql/quarkus/backend/BackendProvider.java
@@ -22,6 +22,7 @@ import static java.lang.String.format;
import jakarta.enterprise.context.ApplicationScoped;
import jakarta.enterprise.inject.Any;
+import jakarta.enterprise.inject.Disposes;
import jakarta.enterprise.inject.Instance;
import jakarta.enterprise.inject.Produces;
import java.util.Optional;
@@ -102,4 +103,8 @@ class BackendProvider {
throw e;
}
}
+
+ void disposeBackend(@Disposes @NotObserved Backend backend) throws Exception {
+ backend.close();
+ }
}