[asterixdb] branch master updated: [ASTERIXDB-2520][RT] Make Dataset Memory Reservation Idempotent

[mh...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

mhubail pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/asterixdb.git


The following commit(s) were added to refs/heads/master by this push:
     new 527f141  [ASTERIXDB-2520][RT] Make Dataset Memory Reservation Idempotent
527f141 is described below

commit 527f141358ed36493b51d74a0516870131e077d8
Author: Murtadha Hubail <mh...@apache.org>
AuthorDate: Fri Feb 15 21:11:15 2019 +0300

    [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 <je...@fulliautomatix.ics.uci.edu>
    Reviewed-by: Michael Blow <mb...@apache.org>
    Integration-Tests: Jenkins <je...@fulliautomatix.ics.uci.edu>
    Tested-by: Jenkins <je...@fulliautomatix.ics.uci.edu>
---
 .../apache/asterix/app/replication/NcLifecycleCoordinator.java    | 6 +++---
 .../org/apache/asterix/common/context/DatasetMemoryManager.java   | 3 ++-
 .../org/apache/asterix/test/context/DatasetMemoryManagerTest.java | 8 --------
 3 files changed, 5 insertions(+), 12 deletions(-)

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 @@ public class NcLifecycleCoordinator implements INcLifecycleCoordinator {
 
     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 @@ public class NcLifecycleCoordinator implements INcLifecycleCoordinator {
     }
 
     @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 @@ public class NcLifecycleCoordinator implements INcLifecycleCoordinator {
         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 @@ public class DatasetMemoryManager implements IDatasetMemoryManager {
     @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 @@ public class DatasetMemoryManagerTest {
         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);