This is an automated email from the ASF dual-hosted git repository.
tkalkirill pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/ignite-3.git
The following commit(s) were added to refs/heads/main by this push:
new e1044be4cd IGNITE-23287 Throw CompactedException for getAll
metastorage methods (#4529)
e1044be4cd is described below
commit e1044be4cdf11b9854c7d1afcd7dca7ce358099a
Author: Kirill Tkalenko <[email protected]>
AuthorDate: Wed Oct 9 15:26:44 2024 +0300
IGNITE-23287 Throw CompactedException for getAll metastorage methods (#4529)
---
.../internal/metastorage/MetaStorageManager.java | 8 +-
.../metastorage/impl/ItMetaStorageServiceTest.java | 2 +-
.../metastorage/impl/MetaStorageManagerImpl.java | 27 +---
.../metastorage/server/KeyValueStorage.java | 24 ++--
.../server/persistence/RocksDbKeyValueStorage.java | 20 +--
.../server/raft/MetaStorageListener.java | 4 +-
.../AbstractCompactionKeyValueStorageTest.java | 136 ++++++++++++++++++++-
.../server/SimpleInMemoryKeyValueStorage.java | 15 ++-
8 files changed, 172 insertions(+), 64 deletions(-)
diff --git
a/modules/metastorage-api/src/main/java/org/apache/ignite/internal/metastorage/MetaStorageManager.java
b/modules/metastorage-api/src/main/java/org/apache/ignite/internal/metastorage/MetaStorageManager.java
index c83eeadc8d..e24cd3877f 100644
---
a/modules/metastorage-api/src/main/java/org/apache/ignite/internal/metastorage/MetaStorageManager.java
+++
b/modules/metastorage-api/src/main/java/org/apache/ignite/internal/metastorage/MetaStorageManager.java
@@ -241,7 +241,13 @@ public interface MetaStorageManager extends
IgniteComponent {
HybridTimestamp timestampByRevisionLocally(long revision);
/**
- * Retrieves entries for given keys.
+ * Returns a future of getting the latest version of entries corresponding
to the given keys from the metastore leader.
+ *
+ * <p>Never completes with a {@link CompactedException}.</p>
+ *
+ * <p>Future may complete with {@link NodeStoppingException} if the node
is in the process of stopping.</p>
+ *
+ * @param keys Set of keys (must not be empty).
*/
CompletableFuture<Map<ByteArray, Entry>> getAll(Set<ByteArray> keys);
diff --git
a/modules/metastorage/src/integrationTest/java/org/apache/ignite/internal/metastorage/impl/ItMetaStorageServiceTest.java
b/modules/metastorage/src/integrationTest/java/org/apache/ignite/internal/metastorage/impl/ItMetaStorageServiceTest.java
index 7e06ecc851..c7cec67a67 100644
---
a/modules/metastorage/src/integrationTest/java/org/apache/ignite/internal/metastorage/impl/ItMetaStorageServiceTest.java
+++
b/modules/metastorage/src/integrationTest/java/org/apache/ignite/internal/metastorage/impl/ItMetaStorageServiceTest.java
@@ -154,7 +154,7 @@ public class ItMetaStorageServiceTest extends
BaseIgniteAbstractTest {
private static final NavigableMap<ByteArray, Entry> EXPECTED_RESULT_MAP;
/** Expected server result collection. */
- private static final Collection<Entry> EXPECTED_SRV_RESULT_COLL;
+ private static final List<Entry> EXPECTED_SRV_RESULT_COLL;
static {
EXPECTED_RESULT_MAP = new TreeMap<>();
diff --git
a/modules/metastorage/src/main/java/org/apache/ignite/internal/metastorage/impl/MetaStorageManagerImpl.java
b/modules/metastorage/src/main/java/org/apache/ignite/internal/metastorage/impl/MetaStorageManagerImpl.java
index 560f5ece69..668ffe73d8 100644
---
a/modules/metastorage/src/main/java/org/apache/ignite/internal/metastorage/impl/MetaStorageManagerImpl.java
+++
b/modules/metastorage/src/main/java/org/apache/ignite/internal/metastorage/impl/MetaStorageManagerImpl.java
@@ -732,32 +732,7 @@ public class MetaStorageManagerImpl implements
MetaStorageManager, MetastorageGr
@Override
public CompletableFuture<Map<ByteArray, Entry>> getAll(Set<ByteArray>
keys) {
- if (!busyLock.enterBusy()) {
- return failedFuture(new NodeStoppingException());
- }
-
- try {
- return metaStorageSvcFut.thenCompose(svc -> svc.getAll(keys));
- } finally {
- busyLock.leaveBusy();
- }
- }
-
- /**
- * Retrieves entries for given keys and the revision upper bound.
- *
- * @see MetaStorageService#getAll(Set, long)
- */
- public CompletableFuture<Map<ByteArray, Entry>> getAll(Set<ByteArray>
keys, long revUpperBound) {
- if (!busyLock.enterBusy()) {
- return failedFuture(new NodeStoppingException());
- }
-
- try {
- return metaStorageSvcFut.thenCompose(svc -> svc.getAll(keys,
revUpperBound));
- } finally {
- busyLock.leaveBusy();
- }
+ return inBusyLock(busyLock, () -> metaStorageSvcFut.thenCompose(svc ->
svc.getAll(keys)));
}
/**
diff --git
a/modules/metastorage/src/main/java/org/apache/ignite/internal/metastorage/server/KeyValueStorage.java
b/modules/metastorage/src/main/java/org/apache/ignite/internal/metastorage/server/KeyValueStorage.java
index ad40c44db3..104c2d8bb4 100644
---
a/modules/metastorage/src/main/java/org/apache/ignite/internal/metastorage/server/KeyValueStorage.java
+++
b/modules/metastorage/src/main/java/org/apache/ignite/internal/metastorage/server/KeyValueStorage.java
@@ -60,8 +60,7 @@ public interface KeyValueStorage extends ManuallyCloseable {
*
* <p>Never throws {@link CompactedException}.</p>
*
- * @param key The key.
- * @return Value corresponding to the given key.
+ * @param key Key.
*/
Entry get(byte[] key);
@@ -112,7 +111,7 @@ public interface KeyValueStorage extends ManuallyCloseable {
* </li>
* </ul>
*
- * @param key The key.
+ * @param key Key.
* @param revUpperBound The upper bound of revision.
* @throws CompactedException If the requested entry was not found and the
{@code revUpperBound} is less than or equal to the last
* {@link #setCompactionRevision compacted} one.
@@ -132,21 +131,24 @@ public interface KeyValueStorage extends
ManuallyCloseable {
List<Entry> get(byte[] key, long revLowerBound, long revUpperBound);
/**
- * Returns all entries corresponding to given keys.
+ * Returns the latest version of entries corresponding to the given keys.
+ *
+ * <p>Never throws {@link CompactedException}.</p>
*
- * @param keys Keys collection.
- * @return Entries corresponding to given keys.
+ * @param keys Not empty keys.
*/
- Collection<Entry> getAll(List<byte[]> keys);
+ List<Entry> getAll(List<byte[]> keys);
/**
- * Returns all entries corresponding to given keys and bounded by the
given revision.
+ * Returns entries corresponding to the given keys and bounded by the
given revision.
*
- * @param keys Keys collection.
+ * @param keys Not empty keys.
* @param revUpperBound Upper bound of revision.
- * @return Entries corresponding to given keys.
+ * @throws CompactedException If getting any of the individual entries
would have thrown this exception as if
+ * {@link #get(byte[], long)} was used.
+ * @see #get(byte[], long)
*/
- Collection<Entry> getAll(List<byte[]> keys, long revUpperBound);
+ List<Entry> getAll(List<byte[]> keys, long revUpperBound);
/**
* Inserts an entry with the given key and given value.
diff --git
a/modules/metastorage/src/main/java/org/apache/ignite/internal/metastorage/server/persistence/RocksDbKeyValueStorage.java
b/modules/metastorage/src/main/java/org/apache/ignite/internal/metastorage/server/persistence/RocksDbKeyValueStorage.java
index a0769a46e3..dbc0c08e97 100644
---
a/modules/metastorage/src/main/java/org/apache/ignite/internal/metastorage/server/persistence/RocksDbKeyValueStorage.java
+++
b/modules/metastorage/src/main/java/org/apache/ignite/internal/metastorage/server/persistence/RocksDbKeyValueStorage.java
@@ -602,7 +602,7 @@ public class RocksDbKeyValueStorage implements
KeyValueStorage {
}
@Override
- public Collection<Entry> getAll(List<byte[]> keys) {
+ public List<Entry> getAll(List<byte[]> keys) {
rwLock.readLock().lock();
try {
@@ -613,7 +613,7 @@ public class RocksDbKeyValueStorage implements
KeyValueStorage {
}
@Override
- public Collection<Entry> getAll(List<byte[]> keys, long revUpperBound) {
+ public List<Entry> getAll(List<byte[]> keys, long revUpperBound) {
rwLock.readLock().lock();
try {
@@ -1046,22 +1046,14 @@ public class RocksDbKeyValueStorage implements
KeyValueStorage {
}
}
- /**
- * Gets all entries with given keys and a revision.
- *
- * @param keys Target keys.
- * @param rev Target revision.
- * @return Collection of entries.
- */
- private Collection<Entry> doGetAll(Collection<byte[]> keys, long rev) {
- assert keys != null : "keys list can't be null.";
- assert !keys.isEmpty() : "keys list can't be empty.";
- assert rev >= 0;
+ private List<Entry> doGetAll(Collection<byte[]> keys, long revUpperBound) {
+ assert !keys.isEmpty();
+ assert revUpperBound >= 0 : revUpperBound;
var res = new ArrayList<Entry>(keys.size());
for (byte[] key : keys) {
- res.add(doGet(key, rev));
+ res.add(doGet(key, revUpperBound));
}
return res;
diff --git
a/modules/metastorage/src/main/java/org/apache/ignite/internal/metastorage/server/raft/MetaStorageListener.java
b/modules/metastorage/src/main/java/org/apache/ignite/internal/metastorage/server/raft/MetaStorageListener.java
index 94c0e6a82a..3829623520 100644
---
a/modules/metastorage/src/main/java/org/apache/ignite/internal/metastorage/server/raft/MetaStorageListener.java
+++
b/modules/metastorage/src/main/java/org/apache/ignite/internal/metastorage/server/raft/MetaStorageListener.java
@@ -24,8 +24,8 @@ import static
org.apache.ignite.internal.util.ByteUtils.toByteArrayList;
import java.io.Serializable;
import java.nio.file.Path;
import java.util.ArrayList;
-import java.util.Collection;
import java.util.Iterator;
+import java.util.List;
import java.util.function.Consumer;
import org.apache.ignite.internal.metastorage.Entry;
import org.apache.ignite.internal.metastorage.MetaStorageManager;
@@ -121,7 +121,7 @@ public class MetaStorageListener implements
RaftGroupListener, BeforeApplyHandle
} else if (command instanceof GetAllCommand) {
GetAllCommand getAllCmd = (GetAllCommand) command;
- Collection<Entry> entries = getAllCmd.revision() ==
MetaStorageManager.LATEST_REVISION
+ List<Entry> entries = getAllCmd.revision() ==
MetaStorageManager.LATEST_REVISION
? storage.getAll(toByteArrayList(getAllCmd.keys()))
:
storage.getAll(toByteArrayList(getAllCmd.keys()), getAllCmd.revision());
diff --git
a/modules/metastorage/src/test/java/org/apache/ignite/internal/metastorage/server/AbstractCompactionKeyValueStorageTest.java
b/modules/metastorage/src/test/java/org/apache/ignite/internal/metastorage/server/AbstractCompactionKeyValueStorageTest.java
index 6bee5c9472..e5c6cca3ac 100644
---
a/modules/metastorage/src/test/java/org/apache/ignite/internal/metastorage/server/AbstractCompactionKeyValueStorageTest.java
+++
b/modules/metastorage/src/test/java/org/apache/ignite/internal/metastorage/server/AbstractCompactionKeyValueStorageTest.java
@@ -18,11 +18,11 @@
package org.apache.ignite.internal.metastorage.server;
import static java.nio.charset.StandardCharsets.UTF_8;
+import static java.util.stream.Collectors.joining;
import static org.apache.ignite.internal.metastorage.dsl.Operations.noop;
import static org.apache.ignite.internal.metastorage.dsl.Operations.ops;
import static org.apache.ignite.internal.metastorage.dsl.Operations.put;
import static org.apache.ignite.internal.metastorage.dsl.Operations.remove;
-import static
org.apache.ignite.internal.metastorage.server.KeyValueStorageUtils.toUtf8String;
import static
org.apache.ignite.internal.testframework.matchers.CompletableFutureMatcher.willCompleteSuccessfully;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
@@ -32,6 +32,7 @@ import static org.junit.jupiter.api.Assertions.assertThrows;
import java.nio.file.Path;
import java.util.ArrayList;
+import java.util.Arrays;
import java.util.List;
import java.util.UUID;
import org.apache.ignite.internal.hlc.HybridClock;
@@ -429,6 +430,110 @@ public abstract class
AbstractCompactionKeyValueStorageTest extends AbstractKeyV
assertDoesNotThrowCompactedExceptionForGetSingleValue(NOT_EXISTS_KEY,
7);
}
+ @Test
+ void testGetAllLatestAndCompaction() {
+ storage.setCompactionRevision(6);
+
+ assertDoesNotThrow(() -> storage.getAll(List.of(FOO_KEY, BAR_KEY,
NOT_EXISTS_KEY)));
+ }
+
+ @Test
+ void testGetAllAndCompactionForFooKey() {
+ // FOO_KEY has revisions: [1, 3, 5].
+ storage.setCompactionRevision(1);
+ assertThrowsCompactedExceptionForGetAll(1, FOO_KEY);
+ assertDoesNotThrowCompactedExceptionForGetAll(2, FOO_KEY);
+
+ storage.setCompactionRevision(2);
+ assertThrowsCompactedExceptionForGetAll(2, FOO_KEY);
+ assertDoesNotThrowCompactedExceptionForGetAll(3, FOO_KEY);
+
+ storage.setCompactionRevision(3);
+ assertThrowsCompactedExceptionForGetAll(3, FOO_KEY);
+ assertDoesNotThrowCompactedExceptionForGetAll(4, FOO_KEY);
+
+ storage.setCompactionRevision(4);
+ assertThrowsCompactedExceptionForGetAll(4, FOO_KEY);
+ assertDoesNotThrowCompactedExceptionForGetAll(5, FOO_KEY);
+
+ storage.setCompactionRevision(5);
+ assertThrowsCompactedExceptionForGetAll(4, FOO_KEY);
+ assertDoesNotThrowCompactedExceptionForGetAll(5, FOO_KEY);
+
+ storage.setCompactionRevision(6);
+ assertThrowsCompactedExceptionForGetAll(4, FOO_KEY);
+ assertDoesNotThrowCompactedExceptionForGetAll(5, FOO_KEY);
+ }
+
+ @Test
+ void testGetAllAndCompactionForBarKey() {
+ // BAR_KEY has revisions: [1, 2, 5 (tombstone)].
+ storage.setCompactionRevision(1);
+ assertThrowsCompactedExceptionForGetAll(1, BAR_KEY);
+ assertDoesNotThrowCompactedExceptionForGetAll(2, BAR_KEY);
+
+ storage.setCompactionRevision(2);
+ assertThrowsCompactedExceptionForGetAll(2, BAR_KEY);
+ assertDoesNotThrowCompactedExceptionForGetAll(3, BAR_KEY);
+
+ storage.setCompactionRevision(3);
+ assertThrowsCompactedExceptionForGetAll(3, BAR_KEY);
+ assertDoesNotThrowCompactedExceptionForGetAll(4, BAR_KEY);
+
+ storage.setCompactionRevision(4);
+ assertThrowsCompactedExceptionForGetAll(4, BAR_KEY);
+ assertDoesNotThrowCompactedExceptionForGetAll(5, BAR_KEY);
+
+ storage.setCompactionRevision(5);
+ assertThrowsCompactedExceptionForGetAll(5, BAR_KEY);
+ assertDoesNotThrowCompactedExceptionForGetAll(6, BAR_KEY);
+
+ storage.setCompactionRevision(6);
+ assertThrowsCompactedExceptionForGetAll(6, BAR_KEY);
+ assertDoesNotThrowCompactedExceptionForGetAll(7, BAR_KEY);
+ }
+
+ @Test
+ void testGetAllAndCompactionForNotExistsKey() {
+ storage.setCompactionRevision(1);
+ assertThrowsCompactedExceptionForGetAll(1, NOT_EXISTS_KEY);
+ assertDoesNotThrowCompactedExceptionForGetAll(2, NOT_EXISTS_KEY);
+
+ storage.setCompactionRevision(2);
+ assertThrowsCompactedExceptionForGetAll(2, NOT_EXISTS_KEY);
+ assertDoesNotThrowCompactedExceptionForGetAll(3, NOT_EXISTS_KEY);
+
+ storage.setCompactionRevision(3);
+ assertThrowsCompactedExceptionForGetAll(3, NOT_EXISTS_KEY);
+ assertDoesNotThrowCompactedExceptionForGetAll(4, NOT_EXISTS_KEY);
+
+ storage.setCompactionRevision(4);
+ assertThrowsCompactedExceptionForGetAll(4, NOT_EXISTS_KEY);
+ assertDoesNotThrowCompactedExceptionForGetAll(5, NOT_EXISTS_KEY);
+
+ storage.setCompactionRevision(5);
+ assertThrowsCompactedExceptionForGetAll(5, NOT_EXISTS_KEY);
+ assertDoesNotThrowCompactedExceptionForGetAll(6, NOT_EXISTS_KEY);
+
+ storage.setCompactionRevision(6);
+ assertThrowsCompactedExceptionForGetAll(6, NOT_EXISTS_KEY);
+ assertDoesNotThrowCompactedExceptionForGetAll(7, NOT_EXISTS_KEY);
+ }
+
+ @Test
+ void testGetAllAndCompactionForMultipleKeys() {
+ storage.setCompactionRevision(1);
+ assertThrowsCompactedExceptionForGetAll(1, FOO_KEY, BAR_KEY,
NOT_EXISTS_KEY);
+ assertDoesNotThrowCompactedExceptionForGetAll(2, FOO_KEY, BAR_KEY,
NOT_EXISTS_KEY);
+
+ storage.setCompactionRevision(5);
+ assertThrowsCompactedExceptionForGetAll(5, FOO_KEY, BAR_KEY,
NOT_EXISTS_KEY);
+ assertThrowsCompactedExceptionForGetAll(5, FOO_KEY, BAR_KEY);
+ assertThrowsCompactedExceptionForGetAll(5, FOO_KEY, NOT_EXISTS_KEY);
+ assertThrowsCompactedExceptionForGetAll(5, BAR_KEY, NOT_EXISTS_KEY);
+ assertDoesNotThrowCompactedExceptionForGetAll(6, FOO_KEY, BAR_KEY,
NOT_EXISTS_KEY);
+ }
+
private List<Integer> collectRevisions(byte[] key) {
var revisions = new ArrayList<Integer>();
@@ -469,4 +574,33 @@ public abstract class
AbstractCompactionKeyValueStorageTest extends AbstractKeyV
);
}
}
+
+ private void assertThrowsCompactedExceptionForGetAll(long
endRevisionInclusive, byte[]... keys) {
+ for (long i = 0; i <= endRevisionInclusive; i++) {
+ long revisionUpperBound = i;
+
+ assertThrows(
+ CompactedException.class,
+ () -> storage.getAll(List.of(keys), revisionUpperBound),
+ () -> String.format("keys=%s, revision=%s",
toUtf8String(keys), revisionUpperBound)
+ );
+ }
+ }
+
+ private void assertDoesNotThrowCompactedExceptionForGetAll(long
startRevisionInclusive, byte[]... keys) {
+ for (long i = startRevisionInclusive; i <= storage.revision(); i++) {
+ long revisionUpperBound = i;
+
+ assertDoesNotThrow(
+ () -> storage.getAll(List.of(keys), revisionUpperBound),
+ () -> String.format("keys=%s, revision=%s",
toUtf8String(keys), revisionUpperBound)
+ );
+ }
+ }
+
+ private static String toUtf8String(byte[]... keys) {
+ return Arrays.stream(keys)
+ .map(KeyValueStorageUtils::toUtf8String)
+ .collect(joining(", ", "[", "]"));
+ }
}
diff --git
a/modules/metastorage/src/testFixtures/java/org/apache/ignite/internal/metastorage/server/SimpleInMemoryKeyValueStorage.java
b/modules/metastorage/src/testFixtures/java/org/apache/ignite/internal/metastorage/server/SimpleInMemoryKeyValueStorage.java
index b92d7a611a..eb0df212ce 100644
---
a/modules/metastorage/src/testFixtures/java/org/apache/ignite/internal/metastorage/server/SimpleInMemoryKeyValueStorage.java
+++
b/modules/metastorage/src/testFixtures/java/org/apache/ignite/internal/metastorage/server/SimpleInMemoryKeyValueStorage.java
@@ -233,14 +233,14 @@ public class SimpleInMemoryKeyValueStorage implements
KeyValueStorage {
}
@Override
- public Collection<Entry> getAll(List<byte[]> keys) {
+ public List<Entry> getAll(List<byte[]> keys) {
synchronized (mux) {
return doGetAll(keys, rev);
}
}
@Override
- public Collection<Entry> getAll(List<byte[]> keys, long revUpperBound) {
+ public List<Entry> getAll(List<byte[]> keys, long revUpperBound) {
synchronized (mux) {
return doGetAll(keys, revUpperBound);
}
@@ -711,15 +711,14 @@ public class SimpleInMemoryKeyValueStorage implements
KeyValueStorage {
}
}
- private Collection<Entry> doGetAll(List<byte[]> keys, long rev) {
- assert keys != null : "keys list can't be null.";
- assert !keys.isEmpty() : "keys list can't be empty.";
- assert rev >= 0;
+ private List<Entry> doGetAll(List<byte[]> keys, long revUpperBound) {
+ assert !keys.isEmpty();
+ assert revUpperBound >= 0 : revUpperBound;
- Collection<Entry> res = new ArrayList<>(keys.size());
+ var res = new ArrayList<Entry>(keys.size());
for (byte[] key : keys) {
- res.add(doGet(key, rev));
+ res.add(doGet(key, revUpperBound));
}
return res;
