This is an automated email from the ASF dual-hosted git repository.
adoroszlai pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/ozone.git
The following commit(s) were added to refs/heads/master by this push:
new 3070f90fea HDDS-8713. Remove RocksDatabase.keyMayExist and the
value.length check. (#4786)
3070f90fea is described below
commit 3070f90fea6fa124ad7c3cb76fa10ccb7a093121
Author: Tsz-Wo Nicholas Sze <[email protected]>
AuthorDate: Tue May 30 02:36:13 2023 +0800
HDDS-8713. Remove RocksDatabase.keyMayExist and the value.length check.
(#4786)
---
.../apache/hadoop/hdds/utils/db/RDBMetrics.java | 8 +++++
.../org/apache/hadoop/hdds/utils/db/RDBTable.java | 38 +++++++++++++---------
.../apache/hadoop/hdds/utils/db/RocksDatabase.java | 31 +++++++-----------
.../hadoop/hdds/utils/db/TestRDBTableStore.java | 17 +++++++++-
4 files changed, 59 insertions(+), 35 deletions(-)
diff --git
a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/RDBMetrics.java
b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/RDBMetrics.java
index ded6007101..a80cb264c5 100644
---
a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/RDBMetrics.java
+++
b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/RDBMetrics.java
@@ -53,6 +53,7 @@ public class RDBMetrics {
private @Metric MutableCounterLong numDBKeyMayExistChecks;
private @Metric MutableCounterLong numDBKeyMayExistMisses;
+ private @Metric MutableCounterLong numDBKeyGets;
private @Metric MutableCounterLong numDBKeyGetIfExistChecks;
private @Metric MutableCounterLong numDBKeyGetIfExistMisses;
private @Metric MutableCounterLong numDBKeyGetIfExistGets;
@@ -60,6 +61,13 @@ public class RDBMetrics {
private @Metric MutableCounterLong walUpdateDataSize;
private @Metric MutableCounterLong walUpdateSequenceCount;
+ public long getNumDBKeyGets() {
+ return numDBKeyGets.value();
+ }
+
+ public void incNumDBKeyGets() {
+ this.numDBKeyGets.incr();
+ }
public long getNumDBKeyGetIfExistGets() {
return numDBKeyGetIfExistGets.value();
diff --git
a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/RDBTable.java
b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/RDBTable.java
index 75ebf40a5c..63d0e431c9 100644
---
a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/RDBTable.java
+++
b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/RDBTable.java
@@ -107,13 +107,17 @@ class RDBTable implements Table<byte[], byte[]> {
@Override
public boolean isExist(byte[] key) throws IOException {
rdbMetrics.incNumDBKeyMayExistChecks();
- final Supplier<byte[]> holder = db.keyMayExistHolder(family, key);
+ final Supplier<byte[]> holder = db.keyMayExist(family, key);
if (holder == null) {
- return false;
+ return false; // definitely not exists
}
final byte[] value = holder.get();
- final boolean exists = (value != null && value.length > 0)
- || db.get(family, key) != null;
+ if (value != null) {
+ return true; // definitely exists
+ }
+
+ // inconclusive: the key may or may not exist
+ final boolean exists = get(key) != null;
if (!exists) {
rdbMetrics.incNumDBKeyMayExistMisses();
}
@@ -122,6 +126,7 @@ class RDBTable implements Table<byte[], byte[]> {
@Override
public byte[] get(byte[] key) throws IOException {
+ rdbMetrics.incNumDBKeyGets();
return db.get(family, key);
}
@@ -145,18 +150,21 @@ class RDBTable implements Table<byte[], byte[]> {
@Override
public byte[] getIfExist(byte[] key) throws IOException {
rdbMetrics.incNumDBKeyGetIfExistChecks();
- final boolean keyMayExist = db.keyMayExist(family, key);
- if (keyMayExist) {
- // Not using out value from string builder, as that is causing
- // IllegalArgumentException during protobuf parsing.
- rdbMetrics.incNumDBKeyGetIfExistGets();
- final byte[] val = db.get(family, key);
- if (val == null) {
- rdbMetrics.incNumDBKeyGetIfExistMisses();
- }
- return val;
+ final Supplier<byte[]> value = db.keyMayExist(family, key);
+ if (value == null) {
+ return null; // definitely not exists
+ }
+ if (value.get() != null) {
+ return value.get(); // definitely exists
+ }
+
+ // inconclusive: the key may or may not exist
+ rdbMetrics.incNumDBKeyGetIfExistGets();
+ final byte[] val = get(key);
+ if (val == null) {
+ rdbMetrics.incNumDBKeyGetIfExistMisses();
}
- return null;
+ return val;
}
@Override
diff --git
a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/RocksDatabase.java
b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/RocksDatabase.java
index 382353ae41..c800cce171 100644
---
a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/RocksDatabase.java
+++
b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/RocksDatabase.java
@@ -645,28 +645,21 @@ public final class RocksDatabase implements Closeable {
}
/**
- * @return false if the key definitely does not exist in the database;
- * otherwise, return true.
- * @see org.rocksdb.RocksDB#keyMayExist(ColumnFamilyHandle, byte[], Holder)
- */
- public boolean keyMayExist(ColumnFamily family, byte[] key)
- throws IOException {
- assertClose();
- try {
- counter.incrementAndGet();
- return db.get().keyMayExist(family.getHandle(), key, null);
- } finally {
- counter.decrementAndGet();
- }
- }
-
- /**
+ * - When the key definitely does not exist in the database,
+ * this method returns null.
+ * - When the key is found in memory,
+ * this method returns a supplier
+ * and {@link Supplier#get()}} returns the value.
+ * - When this method returns a supplier
+ * but {@link Supplier#get()} returns null,
+ * the key may or may not exist in the database.
+ *
* @return the null if the key definitely does not exist in the database;
* otherwise, return a {@link Supplier}.
* @see org.rocksdb.RocksDB#keyMayExist(ColumnFamilyHandle, byte[], Holder)
*/
- public Supplier<byte[]> keyMayExistHolder(ColumnFamily family,
- byte[] key) throws IOException {
+ Supplier<byte[]> keyMayExist(ColumnFamily family, byte[] key)
+ throws IOException {
assertClose();
try {
counter.incrementAndGet();
@@ -686,7 +679,7 @@ public final class RocksDatabase implements Closeable {
return Collections.unmodifiableCollection(columnFamilies.values());
}
- public byte[] get(ColumnFamily family, byte[] key) throws IOException {
+ byte[] get(ColumnFamily family, byte[] key) throws IOException {
assertClose();
try {
counter.incrementAndGet();
diff --git
a/hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/utils/db/TestRDBTableStore.java
b/hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/utils/db/TestRDBTableStore.java
index 4c5061c1c8..62366a9520 100644
---
a/hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/utils/db/TestRDBTableStore.java
+++
b/hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/utils/db/TestRDBTableStore.java
@@ -327,6 +327,8 @@ public class TestRDBTableStore {
.getBytes(StandardCharsets.UTF_8);
byte[] value = RandomStringUtils.random(10, true, false)
.getBytes(StandardCharsets.UTF_8);
+ final byte[] zeroSizeKey = {(byte) (key[0] + 1)};
+ final byte[] zeroSizeValue = {};
final String tableName = families.get(0);
try (Table<byte[], byte[]> testTable = rdbStore.getTable(tableName)) {
@@ -339,6 +341,11 @@ public class TestRDBTableStore {
testTable.delete(key);
Assertions.assertFalse(testTable.isExist(key));
+ // Test a key with zero size value.
+ Assertions.assertNull(testTable.get(zeroSizeKey));
+ testTable.put(zeroSizeKey, zeroSizeValue);
+ Assertions.assertEquals(0, testTable.get(zeroSizeKey).length);
+
byte[] invalidKey =
RandomStringUtils.random(5).getBytes(StandardCharsets.UTF_8);
// Test if isExist returns false for a key that is definitely not
present.
@@ -347,6 +354,7 @@ public class TestRDBTableStore {
RDBMetrics rdbMetrics = rdbStore.getMetrics();
Assertions.assertEquals(3, rdbMetrics.getNumDBKeyMayExistChecks());
Assertions.assertEquals(0, rdbMetrics.getNumDBKeyMayExistMisses());
+ Assertions.assertEquals(2, rdbMetrics.getNumDBKeyGets());
// Reinsert key for further testing.
testTable.put(key, value);
@@ -357,6 +365,13 @@ public class TestRDBTableStore {
try (Table<byte[], byte[]> testTable = rdbStore.getTable(tableName)) {
// Verify isExist works with key not in block cache.
Assertions.assertTrue(testTable.isExist(key));
+ Assertions.assertEquals(0, testTable.get(zeroSizeKey).length);
+ Assertions.assertTrue(testTable.isExist(zeroSizeKey));
+
+ RDBMetrics rdbMetrics = rdbStore.getMetrics();
+ Assertions.assertEquals(2, rdbMetrics.getNumDBKeyMayExistChecks());
+ Assertions.assertEquals(0, rdbMetrics.getNumDBKeyMayExistMisses());
+ Assertions.assertEquals(2, rdbMetrics.getNumDBKeyGets());
}
}
@@ -413,7 +428,7 @@ public class TestRDBTableStore {
Assertions.assertEquals(0, rdbMetrics.getNumDBKeyGetIfExistMisses());
- Assertions.assertEquals(1, rdbMetrics.getNumDBKeyGetIfExistGets());
+ Assertions.assertEquals(0, rdbMetrics.getNumDBKeyGetIfExistGets());
// Reinsert key for further testing.
testTable.put(key, value);
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
