This is an automated email from the ASF dual-hosted git repository.
smengcl 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 8a81f47f661 HDDS-16250. Improve RocksDB read performance by reducing
unnecessary iterator positioning (#11084)
8a81f47f661 is described below
commit 8a81f47f66129283cd6124c178fcf16f48539722
Author: Siyao Meng <[email protected]>
AuthorDate: Sat Aug 22 00:10:15 2026 -0700
HDDS-16250. Improve RocksDB read performance by reducing unnecessary
iterator positioning (#11084)
Generated-by: Codex (GPT-5.6 Sol)
---
.../hdds/utils/db/RDBStoreAbstractIterator.java | 18 ++++++++++++
.../hdds/utils/db/RDBStoreByteArrayIterator.java | 1 -
.../hdds/utils/db/RDBStoreCodecBufferIterator.java | 1 -
.../utils/db/TestRDBStoreByteArrayIterator.java | 29 +++++++++++-------
.../utils/db/TestRDBStoreCodecBufferIterator.java | 34 +++++++++++++++-------
5 files changed, 59 insertions(+), 24 deletions(-)
diff --git
a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/RDBStoreAbstractIterator.java
b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/RDBStoreAbstractIterator.java
index 9ebaf368e9f..15cac80dbd0 100644
---
a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/RDBStoreAbstractIterator.java
+++
b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/RDBStoreAbstractIterator.java
@@ -43,6 +43,10 @@ abstract class RDBStoreAbstractIterator<RAW>
private final RAW prefix;
private final IteratorType type;
private final AtomicBoolean isIteratorClosed = new AtomicBoolean(false);
+ // Defer positioning until the first iterator operation. Explicit positioning
+ // avoids a redundant seek, while ordinary iteration still starts at the
table
+ // or prefix start through initializeIfNeeded().
+ private boolean initialized;
/**
* Constructor for RDBStoreAbstractIterator.
@@ -107,17 +111,27 @@ private void setCurrentEntry() {
}
}
+ private void initializeIfNeeded() {
+ if (!initialized) {
+ // Preserve the original start position for callers that iterate without
+ // explicitly positioning the iterator.
+ seekToFirst();
+ }
+ }
+
@Override
public final boolean hasNext() {
if (isDbClosed()) {
return false;
}
+ initializeIfNeeded();
return rocksDBIterator.get().isValid() &&
(prefix == null || startsWithPrefix(key()));
}
@Override
public final Table.KeyValue<RAW, RAW> next() {
+ initializeIfNeeded();
setCurrentEntry();
if (currentEntry != null) {
rocksDBIterator.get().next();
@@ -133,6 +147,7 @@ public final void seekToFirst() {
} else {
seek0(prefix);
}
+ initialized = true;
setCurrentEntry();
}
@@ -143,12 +158,14 @@ public final void seekToLast() {
} else {
throw new UnsupportedOperationException("seekToLast: prefix != null");
}
+ initialized = true;
setCurrentEntry();
}
@Override
public final Table.KeyValue<RAW, RAW> seek(RAW key) {
seek0(key);
+ initialized = true;
setCurrentEntry();
return currentEntry;
}
@@ -158,6 +175,7 @@ public final void removeFromDB() throws
RocksDatabaseException, CodecException {
if (rocksDBTable == null) {
throw new UnsupportedOperationException("remove");
}
+ initializeIfNeeded();
if (currentEntry != null) {
delete(currentEntry.getKey());
} else {
diff --git
a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/RDBStoreByteArrayIterator.java
b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/RDBStoreByteArrayIterator.java
index 67593f744e3..4238f52fa7c 100644
---
a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/RDBStoreByteArrayIterator.java
+++
b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/RDBStoreByteArrayIterator.java
@@ -31,7 +31,6 @@ private static byte[] copyPrefix(byte[] prefix) {
RDBStoreByteArrayIterator(ManagedRocksIterator iterator,
RDBTable table, byte[] prefix, IteratorType type) {
super(iterator, table, copyPrefix(prefix), type);
- seekToFirst();
}
@Override
diff --git
a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/RDBStoreCodecBufferIterator.java
b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/RDBStoreCodecBufferIterator.java
index aa703249ebe..5430d725651 100644
---
a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/RDBStoreCodecBufferIterator.java
+++
b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/RDBStoreCodecBufferIterator.java
@@ -43,7 +43,6 @@ class RDBStoreCodecBufferIterator extends
RDBStoreAbstractIterator<CodecBuffer>
this.valueBuffer = new Buffer(
new CodecBuffer.Capacity(name + "-iterator-value", 4 << 10),
getType().readValue() ? buffer ->
getRocksDBIterator().get().value(buffer) : null);
- seekToFirst();
}
void assertOpen() {
diff --git
a/hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/utils/db/TestRDBStoreByteArrayIterator.java
b/hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/utils/db/TestRDBStoreByteArrayIterator.java
index 136990522b7..0ca97b9e521 100644
---
a/hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/utils/db/TestRDBStoreByteArrayIterator.java
+++
b/hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/utils/db/TestRDBStoreByteArrayIterator.java
@@ -122,6 +122,7 @@ public void testHasNextDependsOnIsvalid() {
assertTrue(iter.hasNext());
assertFalse(iter.hasNext());
+ verify(rocksDBIteratorMock, times(1)).seekToFirst();
}
@Test
@@ -133,6 +134,7 @@ public void
testNextCallsIsValidThenGetsTheValueAndStepsToNext() {
iter.next();
+ verifier.verify(rocksDBIteratorMock).seekToFirst();
verifier.verify(rocksDBIteratorMock).isValid();
verifier.verify(rocksDBIteratorMock).key();
verifier.verify(rocksDBIteratorMock).value();
@@ -140,10 +142,10 @@ public void
testNextCallsIsValidThenGetsTheValueAndStepsToNext() {
}
@Test
- public void testConstructorSeeksToFirstElement() {
- newIterator();
+ public void testConstructorDoesNotSeek() {
+ newIterator().close();
- verify(rocksDBIteratorMock, times(1)).seekToFirst();
+ verify(rocksDBIteratorMock, never()).seekToFirst();
}
@Test
@@ -152,7 +154,7 @@ public void testSeekToFirstSeeks() {
iter.seekToFirst();
- verify(rocksDBIteratorMock, times(2)).seekToFirst();
+ verify(rocksDBIteratorMock, times(1)).seekToFirst();
}
@Test
@@ -161,23 +163,28 @@ public void testSeekToLastSeeks() {
iter.seekToLast();
+ verify(rocksDBIteratorMock, never()).seekToFirst();
verify(rocksDBIteratorMock, times(1)).seekToLast();
}
@Test
- public void testSeekReturnsTheActualKey() throws Exception {
+ public void testSeekSkipsInitialPrefixSeekAndReturnsActualKey()
+ throws Exception {
when(rocksDBIteratorMock.isValid()).thenReturn(true);
when(rocksDBIteratorMock.key()).thenReturn(new byte[]{0x00});
when(rocksDBIteratorMock.value()).thenReturn(new byte[]{0x7f});
- RDBStoreByteArrayIterator iter = newIterator();
- final Table.KeyValue<byte[], byte[]> val = iter.seek(new byte[]{0x55});
+ RDBStoreByteArrayIterator iter = newIterator(new byte[]{0x11});
+ byte[] target = new byte[]{0x55};
+ final Table.KeyValue<byte[], byte[]> val = iter.seek(target);
InOrder verifier = inOrder(rocksDBIteratorMock);
+ ArgumentCaptor<byte[]> seekKey = forClass(byte[].class);
- verify(rocksDBIteratorMock, times(1)).seekToFirst(); //at construct time
+ verify(rocksDBIteratorMock, never()).seekToFirst();
verify(rocksDBIteratorMock, never()).seekToLast();
- verifier.verify(rocksDBIteratorMock, times(1)).seek(any(byte[].class));
+ verifier.verify(rocksDBIteratorMock, times(1)).seek(seekKey.capture());
+ assertArrayEquals(target, seekKey.getValue());
verifier.verify(rocksDBIteratorMock, times(1)).isValid();
verifier.verify(rocksDBIteratorMock, times(1)).key();
verifier.verify(rocksDBIteratorMock, times(1)).value();
@@ -261,7 +268,7 @@ public void testCloseCloses() throws Exception {
@Test
public void testNullPrefixedIterator() throws IOException {
RDBStoreByteArrayIterator iter = newIterator(null);
- verify(rocksDBIteratorMock, times(1)).seekToFirst();
+ verify(rocksDBIteratorMock, never()).seekToFirst();
clearInvocations(rocksDBIteratorMock);
iter.seekToFirst();
@@ -283,7 +290,7 @@ public void testNullPrefixedIterator() throws IOException {
public void testNormalPrefixedIterator() throws IOException {
byte[] testPrefix = "sample".getBytes(StandardCharsets.UTF_8);
RDBStoreByteArrayIterator iter = newIterator(testPrefix);
- verify(rocksDBIteratorMock, times(1)).seek(testPrefix);
+ verify(rocksDBIteratorMock, never()).seek(any(byte[].class));
clearInvocations(rocksDBIteratorMock);
iter.seekToFirst();
diff --git
a/hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/utils/db/TestRDBStoreCodecBufferIterator.java
b/hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/utils/db/TestRDBStoreCodecBufferIterator.java
index cddb11e9528..e9fddd20966 100644
---
a/hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/utils/db/TestRDBStoreCodecBufferIterator.java
+++
b/hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/utils/db/TestRDBStoreCodecBufferIterator.java
@@ -23,6 +23,7 @@
import static org.junit.jupiter.api.Assertions.assertInstanceOf;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.mockito.ArgumentCaptor.forClass;
import static org.mockito.Mockito.any;
import static org.mockito.Mockito.clearInvocations;
import static org.mockito.Mockito.inOrder;
@@ -45,6 +46,7 @@
import org.apache.log4j.Logger;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
+import org.mockito.ArgumentCaptor;
import org.mockito.InOrder;
import org.mockito.stubbing.Answer;
import org.rocksdb.RocksIterator;
@@ -138,6 +140,7 @@ public void testHasNextDependsOnIsvalid() throws Exception {
assertTrue(i.hasNext());
assertFalse(i.hasNext());
}
+ verify(rocksIteratorMock, times(1)).seekToFirst();
CodecTestUtil.gc();
}
@@ -151,6 +154,7 @@ public void
testNextCallsIsValidThenGetsTheValueAndStepsToNext()
i.next();
}
+ verifier.verify(rocksIteratorMock).seekToFirst();
verifier.verify(rocksIteratorMock).isValid();
verifier.verify(rocksIteratorMock).key(any(ByteBuffer.class));
verifier.verify(rocksIteratorMock).value(any(ByteBuffer.class));
@@ -160,10 +164,10 @@ public void
testNextCallsIsValidThenGetsTheValueAndStepsToNext()
}
@Test
- public void testConstructorSeeksToFirstElement() throws Exception {
+ public void testConstructorDoesNotSeek() throws Exception {
newIterator().close();
- verify(rocksIteratorMock, times(1)).seekToFirst();
+ verify(rocksIteratorMock, never()).seekToFirst();
CodecTestUtil.gc();
}
@@ -173,7 +177,7 @@ public void testSeekToFirstSeeks() throws Exception {
try (RDBStoreCodecBufferIterator i = newIterator()) {
i.seekToFirst();
}
- verify(rocksIteratorMock, times(2)).seekToFirst();
+ verify(rocksIteratorMock, times(1)).seekToFirst();
CodecTestUtil.gc();
}
@@ -184,29 +188,37 @@ public void testSeekToLastSeeks() throws Exception {
i.seekToLast();
}
+ verify(rocksIteratorMock, never()).seekToFirst();
verify(rocksIteratorMock, times(1)).seekToLast();
CodecTestUtil.gc();
}
@Test
- public void testSeekReturnsTheActualKey() throws Exception {
+ public void testSeekSkipsInitialPrefixSeekAndReturnsActualKey()
+ throws Exception {
when(rocksIteratorMock.isValid()).thenReturn(true);
when(rocksIteratorMock.key(any(ByteBuffer.class)))
.then(newAnswerInt("key1", 0x00));
when(rocksIteratorMock.value(any(ByteBuffer.class)))
.then(newAnswerInt("val1", 0x7f));
- try (RDBStoreCodecBufferIterator i = newIterator();
- CodecBuffer target = CodecBuffer.wrap(new byte[]{0x55})) {
+ byte[] targetBytes = new byte[]{0x55};
+ try (RDBStoreCodecBufferIterator i = newIterator(
+ CodecBuffer.wrap(new byte[]{0x11}));
+ CodecBuffer target = CodecBuffer.wrap(targetBytes)) {
final Table.KeyValue<CodecBuffer, CodecBuffer> val = i.seek(target);
InOrder verifier = inOrder(rocksIteratorMock);
+ ArgumentCaptor<ByteBuffer> seekKey = forClass(ByteBuffer.class);
- verify(rocksIteratorMock, times(1)).seekToFirst(); //at construct time
+ verify(rocksIteratorMock, never()).seekToFirst();
verify(rocksIteratorMock, never()).seekToLast();
- verifier.verify(rocksIteratorMock, times(1))
- .seek(any(ByteBuffer.class));
+ verifier.verify(rocksIteratorMock, times(1)).seek(seekKey.capture());
+ ByteBuffer actualSeekKey = seekKey.getValue().duplicate();
+ byte[] actualSeekKeyBytes = new byte[actualSeekKey.remaining()];
+ actualSeekKey.get(actualSeekKeyBytes);
+ assertArrayEquals(targetBytes, actualSeekKeyBytes);
verifier.verify(rocksIteratorMock, times(1)).isValid();
verifier.verify(rocksIteratorMock, times(1)).key(any(ByteBuffer.class));
verifier.verify(rocksIteratorMock,
times(1)).value(any(ByteBuffer.class));
@@ -310,7 +322,7 @@ public void testCloseCloses() throws Exception {
@Test
public void testNullPrefixedIterator() throws Exception {
try (RDBStoreCodecBufferIterator i = newIterator()) {
- verify(rocksIteratorMock, times(1)).seekToFirst();
+ verify(rocksIteratorMock, never()).seekToFirst();
clearInvocations(rocksIteratorMock);
i.seekToFirst();
@@ -335,7 +347,7 @@ public void testNormalPrefixedIterator() throws Exception {
try (RDBStoreCodecBufferIterator i = newIterator(
CodecBuffer.wrap(prefixBytes))) {
final ByteBuffer prefix = ByteBuffer.wrap(prefixBytes);
- verify(rocksIteratorMock, times(1)).seek(prefix);
+ verify(rocksIteratorMock, never()).seek(any(ByteBuffer.class));
clearInvocations(rocksIteratorMock);
i.seekToFirst();
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
