Murtadha Hubail has submitted this change and it was merged. Change subject: [ASTERIXDB-2520][RT] Make Dataset Memory Reservation Idempotent ......................................................................
[ASTERIXDB-2520][RT] Make Dataset Memory Reservation Idempotent - user model changes: no - storage format changes: no - interface changes: no Details: - Ensure thread-safety of metadata node takeover. - To account for multiple failures of metadata node, make dataset memory reservation idempotent. Change-Id: I360226187e176ce3a0ccdcd7a1b611a01d906394 Reviewed-on: https://asterix-gerrit.ics.uci.edu/3207 Sonar-Qube: Jenkins <[email protected]> Reviewed-by: Michael Blow <[email protected]> Integration-Tests: Jenkins <[email protected]> Tested-by: Jenkins <[email protected]> --- M asterixdb/asterix-app/src/main/java/org/apache/asterix/app/replication/NcLifecycleCoordinator.java M asterixdb/asterix-common/src/main/java/org/apache/asterix/common/context/DatasetMemoryManager.java M asterixdb/asterix-common/src/test/java/org/apache/asterix/test/context/DatasetMemoryManagerTest.java 3 files changed, 5 insertions(+), 12 deletions(-) Approvals: Jenkins: Verified; No violations found; Verified Michael Blow: Looks good to me, approved Objections: Anon. E. Moose #1000171: Violations found Jenkins: Violations found diff --git a/asterixdb/asterix-app/src/main/java/org/apache/asterix/app/replication/NcLifecycleCoordinator.java b/asterixdb/asterix-app/src/main/java/org/apache/asterix/app/replication/NcLifecycleCoordinator.java index 961593f..19ae171 100644 --- a/asterixdb/asterix-app/src/main/java/org/apache/asterix/app/replication/NcLifecycleCoordinator.java +++ b/asterixdb/asterix-app/src/main/java/org/apache/asterix/app/replication/NcLifecycleCoordinator.java @@ -60,7 +60,7 @@ private static final Logger LOGGER = LogManager.getLogger(); protected IClusterStateManager clusterManager; - protected String metadataNodeId; + protected volatile String metadataNodeId; protected Set<String> pendingStartupCompletionNodes = new HashSet<>(); protected final ICCMessageBroker messageBroker; private final boolean replicationEnabled; @@ -157,7 +157,7 @@ } @Override - public void notifyMetadataNodeChange(String node) throws HyracksDataException { + public synchronized void notifyMetadataNodeChange(String node) throws HyracksDataException { if (metadataNodeId.equals(node)) { return; } @@ -203,7 +203,7 @@ return tasks; } - private void process(MetadataNodeResponseMessage response) throws HyracksDataException { + private synchronized void process(MetadataNodeResponseMessage response) throws HyracksDataException { // rebind metadata node since it might be changing MetadataManager.INSTANCE.rebindMetadataNode(); clusterManager.updateMetadataNode(response.getNodeId(), response.isExported()); diff --git a/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/context/DatasetMemoryManager.java b/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/context/DatasetMemoryManager.java index 1b4b48e..8aec240 100644 --- a/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/context/DatasetMemoryManager.java +++ b/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/context/DatasetMemoryManager.java @@ -82,7 +82,8 @@ @Override public synchronized boolean reserve(int datasetId) { if (reservedMap.containsKey(datasetId)) { - throw new IllegalStateException("Memory is already reserved for dataset: " + datasetId); + LOGGER.info("Memory is already reserved for dataset: {}", () -> datasetId); + return true; } final long required = getTotalSize(datasetId); if (!isAllocatable(required) && !allocatedMap.containsKey(datasetId)) { diff --git a/asterixdb/asterix-common/src/test/java/org/apache/asterix/test/context/DatasetMemoryManagerTest.java b/asterixdb/asterix-common/src/test/java/org/apache/asterix/test/context/DatasetMemoryManagerTest.java index b8e3604..2a26f08 100644 --- a/asterixdb/asterix-common/src/test/java/org/apache/asterix/test/context/DatasetMemoryManagerTest.java +++ b/asterixdb/asterix-common/src/test/java/org/apache/asterix/test/context/DatasetMemoryManagerTest.java @@ -83,15 +83,7 @@ Assert.assertEquals(memoryManager.getAvailable(), expectedBudget); // double reserve - boolean thrown = false; Assert.assertTrue(memoryManager.reserve(2)); - try { - memoryManager.reserve(2); - } catch (IllegalStateException e) { - Assert.assertTrue(e.getMessage().contains("already reserved")); - thrown = true; - } - Assert.assertTrue(thrown); // cancel reserved memoryManager.cancelReserved(2); -- To view, visit https://asterix-gerrit.ics.uci.edu/3207 To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-MessageType: merged Gerrit-Change-Id: I360226187e176ce3a0ccdcd7a1b611a01d906394 Gerrit-PatchSet: 5 Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-Owner: Murtadha Hubail <[email protected]> Gerrit-Reviewer: Anon. E. Moose #1000171 Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-Reviewer: Michael Blow <[email protected]> Gerrit-Reviewer: Murtadha Hubail <[email protected]>
