You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by GitBox <gi...@apache.org> on 2021/05/06 16:51:02 UTC

[GitHub] [ignite-3] alievmirza commented on a change in pull request #114: IGNITE-14666 Add proper listener handling in DistributedConfigurationStorage and LocalConfigurationStorage

alievmirza commented on a change in pull request #114:
URL: https://github.com/apache/ignite-3/pull/114#discussion_r627594791



##########
File path: modules/runner/src/main/java/org/apache/ignite/internal/storage/DistributedConfigurationStorage.java
##########
@@ -49,52 +72,152 @@ public DistributedConfigurationStorage(MetaStorageManager metaStorageMgr) {
         this.metaStorageMgr = metaStorageMgr;
     }
 
-    /** Map to store values. */
-    private Map<String, Serializable> map = new ConcurrentHashMap<>();
-
     /** Change listeners. */
     private List<ConfigurationStorageListener> listeners = new CopyOnWriteArrayList<>();
 
-    /** Storage version. */
-    private AtomicLong version = new AtomicLong(0);
+    /** Storage version. It stores actual metastorage revision, that is applied to configuration manager. */
+    private AtomicLong version = new AtomicLong(0L);
 
     /** {@inheritDoc} */
     @Override public synchronized Data readAll() throws StorageException {
-        return new Data(new HashMap<>(map), version.get(), 0);
+
+        Cursor<Entry> cur = metaStorageMgr.rangeWithAppliedRevision(new Key(DISTRIBUTED_PREFIX + "."),

Review comment:
       yes, fixed

##########
File path: modules/runner/src/main/java/org/apache/ignite/internal/storage/DistributedConfigurationStorage.java
##########
@@ -49,52 +72,152 @@ public DistributedConfigurationStorage(MetaStorageManager metaStorageMgr) {
         this.metaStorageMgr = metaStorageMgr;
     }
 
-    /** Map to store values. */
-    private Map<String, Serializable> map = new ConcurrentHashMap<>();
-
     /** Change listeners. */
     private List<ConfigurationStorageListener> listeners = new CopyOnWriteArrayList<>();
 
-    /** Storage version. */
-    private AtomicLong version = new AtomicLong(0);
+    /** Storage version. It stores actual metastorage revision, that is applied to configuration manager. */
+    private AtomicLong version = new AtomicLong(0L);
 
     /** {@inheritDoc} */
     @Override public synchronized Data readAll() throws StorageException {
-        return new Data(new HashMap<>(map), version.get(), 0);
+
+        Cursor<Entry> cur = metaStorageMgr.rangeWithAppliedRevision(new Key(DISTRIBUTED_PREFIX + "."),
+            new Key(DISTRIBUTED_PREFIX + (char)('.' + 1)));
+
+        HashMap<String, Serializable> data = new HashMap<>();
+
+        long maxRevision = 0L;

Review comment:
       I compute maxRevision to make an assertion

##########
File path: modules/runner/src/main/java/org/apache/ignite/internal/storage/DistributedConfigurationStorage.java
##########
@@ -49,52 +72,152 @@ public DistributedConfigurationStorage(MetaStorageManager metaStorageMgr) {
         this.metaStorageMgr = metaStorageMgr;
     }
 
-    /** Map to store values. */
-    private Map<String, Serializable> map = new ConcurrentHashMap<>();
-
     /** Change listeners. */
     private List<ConfigurationStorageListener> listeners = new CopyOnWriteArrayList<>();
 
-    /** Storage version. */
-    private AtomicLong version = new AtomicLong(0);
+    /** Storage version. It stores actual metastorage revision, that is applied to configuration manager. */
+    private AtomicLong version = new AtomicLong(0L);
 
     /** {@inheritDoc} */
     @Override public synchronized Data readAll() throws StorageException {
-        return new Data(new HashMap<>(map), version.get(), 0);
+
+        Cursor<Entry> cur = metaStorageMgr.rangeWithAppliedRevision(new Key(DISTRIBUTED_PREFIX + "."),
+            new Key(DISTRIBUTED_PREFIX + (char)('.' + 1)));

Review comment:
       done

##########
File path: modules/runner/src/main/java/org/apache/ignite/internal/storage/DistributedConfigurationStorage.java
##########
@@ -49,52 +72,152 @@ public DistributedConfigurationStorage(MetaStorageManager metaStorageMgr) {
         this.metaStorageMgr = metaStorageMgr;
     }
 
-    /** Map to store values. */
-    private Map<String, Serializable> map = new ConcurrentHashMap<>();
-
     /** Change listeners. */
     private List<ConfigurationStorageListener> listeners = new CopyOnWriteArrayList<>();
 
-    /** Storage version. */
-    private AtomicLong version = new AtomicLong(0);
+    /** Storage version. It stores actual metastorage revision, that is applied to configuration manager. */
+    private AtomicLong version = new AtomicLong(0L);
 
     /** {@inheritDoc} */
     @Override public synchronized Data readAll() throws StorageException {
-        return new Data(new HashMap<>(map), version.get(), 0);
+
+        Cursor<Entry> cur = metaStorageMgr.rangeWithAppliedRevision(new Key(DISTRIBUTED_PREFIX + "."),
+            new Key(DISTRIBUTED_PREFIX + (char)('.' + 1)));
+
+        HashMap<String, Serializable> data = new HashMap<>();
+
+        long maxRevision = 0L;
+
+        Entry entryForMasterKey = null;
+
+        for (Entry entry : cur) {
+            if (!entry.key().equals(masterKey)) {
+                data.put(entry.key().toString().replaceFirst(DISTRIBUTED_PREFIX + ".", ""),

Review comment:
       done

##########
File path: modules/runner/src/main/java/org/apache/ignite/internal/storage/LocalConfigurationStorage.java
##########
@@ -49,47 +69,121 @@ public LocalConfigurationStorage(VaultManager vaultMgr) {
         this.vaultMgr = vaultMgr;
     }
 
-    /** Map to store values. */
-    private Map<String, Serializable> map = new ConcurrentHashMap<>();
-
     /** Change listeners. */
     private List<ConfigurationStorageListener> listeners = new CopyOnWriteArrayList<>();
 
     /** Storage version. */
     private AtomicLong version = new AtomicLong(0);
 
+    /** Start key in ragne for searching local configuration keys. */
+    private ByteArray localKeysStartRange = ByteArray.fromString(LOCAL_PREFIX + ".");
+
+    /** End key in range for searching local configuration keys. */
+    private ByteArray localKeysEndRange = ByteArray.fromString(LOCAL_PREFIX + (char)('.' + 1));
+
     /** {@inheritDoc} */
     @Override public synchronized Data readAll() throws StorageException {
-        return new Data(new HashMap<>(map), version.get(), 0);
+        Iterator<Entry> iter =
+            vaultMgr.range(localKeysStartRange, localKeysEndRange);
+
+        HashMap<String, Serializable> data = new HashMap<>();
+
+        while (iter.hasNext()) {
+            Entry val = iter.next();
+
+            data.put(val.key().toString().replaceFirst(LOCAL_PREFIX + ".", ""),
+                (Serializable)ByteUtils.fromBytes(val.value()));
+        }
+
+        return new Data(data, version.get());

Review comment:
       yes, this will be needed when restart is developed

##########
File path: modules/runner/src/main/java/org/apache/ignite/internal/storage/DistributedConfigurationStorage.java
##########
@@ -49,52 +72,152 @@ public DistributedConfigurationStorage(MetaStorageManager metaStorageMgr) {
         this.metaStorageMgr = metaStorageMgr;
     }
 
-    /** Map to store values. */
-    private Map<String, Serializable> map = new ConcurrentHashMap<>();
-
     /** Change listeners. */
     private List<ConfigurationStorageListener> listeners = new CopyOnWriteArrayList<>();
 
-    /** Storage version. */
-    private AtomicLong version = new AtomicLong(0);
+    /** Storage version. It stores actual metastorage revision, that is applied to configuration manager. */
+    private AtomicLong version = new AtomicLong(0L);
 
     /** {@inheritDoc} */
     @Override public synchronized Data readAll() throws StorageException {
-        return new Data(new HashMap<>(map), version.get(), 0);
+
+        Cursor<Entry> cur = metaStorageMgr.rangeWithAppliedRevision(new Key(DISTRIBUTED_PREFIX + "."),
+            new Key(DISTRIBUTED_PREFIX + (char)('.' + 1)));
+
+        HashMap<String, Serializable> data = new HashMap<>();
+
+        long maxRevision = 0L;
+
+        Entry entryForMasterKey = null;
+
+        for (Entry entry : cur) {
+            if (!entry.key().equals(masterKey)) {
+                data.put(entry.key().toString().replaceFirst(DISTRIBUTED_PREFIX + ".", ""),
+                    (Serializable)ByteUtils.fromBytes(entry.value()));
+
+                if (maxRevision < entry.revision())
+                    maxRevision = entry.revision();
+            } else
+                entryForMasterKey = entry;
+        }
+
+        if (!data.isEmpty()) {
+            assert entryForMasterKey != null;
+
+            assert maxRevision == entryForMasterKey.revision();
+
+            assert maxRevision >= version.get();
+
+            return new Data(data, maxRevision);
+        }
+
+        return new Data(data, version.get());
     }
 
     /** {@inheritDoc} */
-    @Override public synchronized CompletableFuture<Boolean> write(Map<String, Serializable> newValues, long version) {
-        if (version != this.version.get())
+    @Override public synchronized CompletableFuture<Boolean> write(Map<String, Serializable> newValues, long sentVersion) {
+        assert sentVersion <= version.get();
+
+        if (sentVersion != version.get())
+            // This means that sentVersion is less than version and other node has already updated configuration and
+            // write should be retried. Actual version will be set when watch and corresponding configuration listener
+            // updates configuration and notifyApplied is triggered afterwards.
             return CompletableFuture.completedFuture(false);
 
+        HashSet<Operation> operations = new HashSet<>();
+
+        HashSet<Operation> failures = new HashSet<>();
+
         for (Map.Entry<String, Serializable> entry : newValues.entrySet()) {
+            Key key = new Key(DISTRIBUTED_PREFIX + "." + entry.getKey());
+
             if (entry.getValue() != null)
-                map.put(entry.getKey(), entry.getValue());
+                operations.add(Operations.put(key, ByteUtils.toBytes(entry.getValue())));
             else
-                map.remove(entry.getKey());
-        }
+                operations.add(Operations.remove(key));
 
-        this.version.incrementAndGet();
+            failures.add(Operations.noop());
+        }
 
-        listeners.forEach(listener -> listener.onEntriesChanged(new Data(newValues, this.version.get(), 0)));
+        operations.add(Operations.put(masterKey, new byte[1]));
 
-        return CompletableFuture.completedFuture(true);
+        return metaStorageMgr.invoke(Conditions.key(masterKey).revision().eq(version.get()), operations, failures);
     }
 
     /** {@inheritDoc} */
-    @Override public void addListener(ConfigurationStorageListener listener) {
+    @Override public synchronized void addListener(ConfigurationStorageListener listener) {
         listeners.add(listener);
+
+        if (watchId == null) {
+            watchId = metaStorageMgr.registerWatchByPrefix(masterKey, new WatchListener() {
+                @Override public boolean onUpdate(@NotNull Iterable<WatchEvent> events) {
+                    HashMap<String, Serializable> data = new HashMap<>();
+
+                    long maxRevision = 0L;
+
+                    Entry entryForMasterKey = null;
+
+                    for (WatchEvent event : events) {
+                        Entry e = event.newEntry();
+
+                        if (!e.key().equals(masterKey)) {
+                            data.put(e.key().toString().replaceFirst(DISTRIBUTED_PREFIX + ".", ""),

Review comment:
       done

##########
File path: modules/runner/src/main/java/org/apache/ignite/internal/storage/LocalConfigurationStorage.java
##########
@@ -49,47 +69,121 @@ public LocalConfigurationStorage(VaultManager vaultMgr) {
         this.vaultMgr = vaultMgr;
     }
 
-    /** Map to store values. */
-    private Map<String, Serializable> map = new ConcurrentHashMap<>();
-
     /** Change listeners. */
     private List<ConfigurationStorageListener> listeners = new CopyOnWriteArrayList<>();
 
     /** Storage version. */
     private AtomicLong version = new AtomicLong(0);
 
+    /** Start key in ragne for searching local configuration keys. */
+    private ByteArray localKeysStartRange = ByteArray.fromString(LOCAL_PREFIX + ".");
+
+    /** End key in range for searching local configuration keys. */
+    private ByteArray localKeysEndRange = ByteArray.fromString(LOCAL_PREFIX + (char)('.' + 1));

Review comment:
       done




-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org