[couchdb] 01/01: fix race condition when creating indexes

[rn...@apache.org]

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

rnewson pushed a commit to branch nouveau-race-condition-creating-index
in repository https://gitbox.apache.org/repos/asf/couchdb.git

commit 8071154546a582910618b2a410288d4e75e07068
Author: Robert Newson <rn...@apache.org>
AuthorDate: Thu Apr 27 22:35:16 2023 +0100

    fix race condition when creating indexes
---
 .../apache/couchdb/nouveau/core/IndexManager.java  | 46 +++++++++++++++-------
 .../apache/couchdb/nouveau/core/StripedLock.java   | 44 +++++++++++++++++++++
 2 files changed, 75 insertions(+), 15 deletions(-)

diff --git a/nouveau/src/main/java/org/apache/couchdb/nouveau/core/IndexManager.java b/nouveau/src/main/java/org/apache/couchdb/nouveau/core/IndexManager.java
index f662780bb..0ddaa24ce 100644
--- a/nouveau/src/main/java/org/apache/couchdb/nouveau/core/IndexManager.java
+++ b/nouveau/src/main/java/org/apache/couchdb/nouveau/core/IndexManager.java
@@ -24,6 +24,7 @@ import java.time.Duration;
 import java.util.List;
 import java.util.concurrent.ScheduledExecutorService;
 import java.util.concurrent.TimeUnit;
+import java.util.concurrent.locks.Lock;
 import java.util.stream.Stream;
 
 import org.apache.couchdb.nouveau.api.IndexDefinition;
@@ -75,6 +76,8 @@ public final class IndexManager implements Managed {
 
     private Cache<String, Index> cache;
 
+    private StripedLock<String> lock;
+
     public <R> R with(final String name, final IndexLoader loader, final IndexFunction<Index, R> indexFun)
             throws IOException, InterruptedException {
         while (true) {
@@ -125,26 +128,32 @@ public final class IndexManager implements Managed {
         }
     }
 
-    public void create(final String name, IndexDefinition newIndexDefinition) throws IOException {
-        if (exists(name)) {
-            final IndexDefinition currentIndexDefinition = loadIndexDefinition(name);
-            if (newIndexDefinition.equals(currentIndexDefinition)) {
-                // Idempotent success.
+    public void create(final String name, IndexDefinition indexDefinition) throws IOException {
+        final Lock readLock = lock.readLock(name);
+        final Lock writeLock = lock.writeLock(name);
+
+        readLock.lock();
+        try {
+            if (exists(name)) {
+                assertSame(indexDefinition, loadIndexDefinition(name));
                 return;
             }
-            throw new WebApplicationException("Index already exists", Status.EXPECTATION_FAILED);
+        } finally {
+            readLock.unlock();
         }
-        // Validate index definiton
-        // TODO luceneFor(indexDefinition).validate(indexDefinition);
-
-        // Persist definition
-        final Path path = indexDefinitionPath(name);
 
-        if (Files.exists(path)) {
-            throw new FileAlreadyExistsException(name + " already exists");
+        writeLock.lock();
+        try {
+            if (exists(name)) {
+                assertSame(indexDefinition, loadIndexDefinition(name));
+                return;
+            }
+            final Path path = indexDefinitionPath(name);
+            Files.createDirectories(path.getParent());
+            objectMapper.writeValue(path.toFile(), indexDefinition);
+        } finally {
+            writeLock.unlock();
         }
-        Files.createDirectories(path.getParent());
-        objectMapper.writeValue(path.toFile(), newIndexDefinition);
     }
 
     public boolean exists(final String name) {
@@ -251,6 +260,7 @@ public final class IndexManager implements Managed {
                 .scheduler(Scheduler.systemScheduler())
                 .evictionListener(new IndexEvictionListener())
                 .build();
+        lock = new StripedLock<String>(100);
     }
 
     @Override
@@ -324,4 +334,10 @@ public final class IndexManager implements Managed {
                 });
     }
 
+    private void assertSame(final IndexDefinition a, final IndexDefinition b) {
+        if (!a.equals(b)) {
+            throw new WebApplicationException("Index already exists", Status.EXPECTATION_FAILED);
+        }
+    }
+
 }
diff --git a/nouveau/src/main/java/org/apache/couchdb/nouveau/core/StripedLock.java b/nouveau/src/main/java/org/apache/couchdb/nouveau/core/StripedLock.java
new file mode 100644
index 000000000..ad2948ee7
--- /dev/null
+++ b/nouveau/src/main/java/org/apache/couchdb/nouveau/core/StripedLock.java
@@ -0,0 +1,44 @@
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package org.apache.couchdb.nouveau.core;
+
+import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReadWriteLock;
+import java.util.concurrent.locks.ReentrantReadWriteLock;
+
+final class StripedLock<K> {
+
+    private final ReadWriteLock[] locks;
+
+    public StripedLock(
+            final int lockCount) {
+        this.locks = new ReadWriteLock[lockCount];
+        for (int i = 0; i < locks.length; i++) {
+            this.locks[i] = new ReentrantReadWriteLock();
+        }
+    }
+
+    public Lock readLock(final K key) {
+        return readWriteLock(key).readLock();
+    }
+
+    public Lock writeLock(final K key) {
+        return readWriteLock(key).writeLock();
+    }
+
+    private ReadWriteLock readWriteLock(final K key) {
+        return locks[Math.floorMod(key.hashCode(), locks.length)];
+    }
+
+}