You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "SaketaChalamchala (via GitHub)" <gi...@apache.org> on 2026/08/20 00:30:05 UTC
[PR] HDDS-16229. Add RocksDB multiGetSkipCache for batched table reads. [ozone]
SaketaChalamchala opened a new pull request, #11068:
URL: https://github.com/apache/ozone/pull/11068
## What changes were proposed in this pull request?
RocksDB `MultiGet` support through `RocksDatabase`, `RDBTable`, and `TypedTable` on a skip-cache path. Callers can batch point lookups in one RocksDB call instead of looping `get()` / `getSkipCache()`.
Missing keys return null at the matching list index; order matches the input key list.
**Out of scope (follow-ups):** cache-aware Table.multiGet, batch metrics, CodecBuffer fast path, call-site adoption.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-16229
## How was this patch tested?
Unit Tests
--
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.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-16229. Add RocksDB multiGetSkipCache for batched table reads. [ozone]
Posted by "SaketaChalamchala (via GitHub)" <gi...@apache.org>.
SaketaChalamchala commented on PR #11068:
URL: https://github.com/apache/ozone/pull/11068#issuecomment-5458815807
> @SaketaChalamchala — I took a pass at refactoring `TypedTable.multiGetFromTable()` to address the duplicated status handling between the first pass and the oversize retry loop.
>
> **Branch:** https://github.com/jojochuang/ozone/tree/HDDS-16229-refactor **Commit:** [jojochuang@0504b5bc8e4](https://github.com/jojochuang/ozone/commit/0504b5bc8e4)
Thanks for the review and refactor @jojochuang. I took the refactor and added minor updates to update the bufferCapacity hint for the table, the null guards and addressed leaks due to partial value buffer close if there is an exception.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-16229. Add RocksDB multiGetSkipCache for batched table reads. [ozone]
Posted by "jojochuang (via GitHub)" <gi...@apache.org>.
jojochuang commented on code in PR #11068:
URL: https://github.com/apache/ozone/pull/11068#discussion_r3866269545
##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/TypedTable.java:
##########
@@ -228,6 +230,15 @@ public VALUE getSkipCache(KEY key) throws RocksDatabaseException, CodecException
return getFromTable(key);
}
+ @Override
+ public List<VALUE> multiGetSkipCache(List<KEY> keys)
+ throws RocksDatabaseException, CodecException {
+ if (keys.isEmpty()) {
Review Comment:
Suggest to guard against null keys to be consistent with Table.multiGetSkipCache()
```suggestion
if (keys == null || keys.isEmpty()) {
```
##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/TypedTable.java:
##########
@@ -306,6 +317,114 @@ private VALUE getFromTable(KEY key) throws RocksDatabaseException, CodecExceptio
}
}
+ List<VALUE> multiGetFromTable(List<KEY> keys)
+ throws RocksDatabaseException, CodecException {
+ if (supportCodecBuffer) {
Review Comment:
i would refactor here to make it more readable. Too much things happen in this method.
##########
hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/utils/db/InMemoryTestTable.java:
##########
@@ -75,6 +76,18 @@ public VALUE get(KEY key) {
return map.get(key);
}
+ @Override
+ public List<VALUE> multiGetSkipCache(List<KEY> keys) {
+ if (keys.isEmpty()) {
Review Comment:
```suggestion
if (keys == null || keys.isEmpty()) {
```
--
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.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-16229. Add RocksDB multiGetSkipCache for batched table reads. [ozone]
Posted by "jojochuang (via GitHub)" <gi...@apache.org>.
jojochuang commented on PR #11068:
URL: https://github.com/apache/ozone/pull/11068#issuecomment-5621421608
Merged. Thanks @SaketaChalamchala @swamirishi @yandrey321
--
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.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-16229. Add RocksDB multiGetSkipCache for batched table reads. [ozone]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #11068:
URL: https://github.com/apache/ozone/pull/11068#discussion_r3899865788
##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/TypedTable.java:
##########
@@ -306,6 +317,143 @@ private VALUE getFromTable(KEY key) throws RocksDatabaseException, CodecExceptio
}
}
+ List<VALUE> multiGetFromTable(List<KEY> keys)
+ throws RocksDatabaseException, CodecException {
+ if (supportCodecBuffer) {
+ return multiGetFromTableWithCodecBuffer(keys);
+ }
+ return multiGetFromTableWithByteArray(keys);
+ }
+
+ List<VALUE> multiGetFromTableWithByteArray(List<KEY> keys)
+ throws RocksDatabaseException, CodecException {
+ List<byte[]> keyBytesList = new ArrayList<>(keys.size());
+ for (KEY key : keys) {
+ keyBytesList.add(encodeKey(key));
+ }
+ List<byte[]> valueBytesList = rawTable.multiGetSkipCache(keyBytesList);
+ List<VALUE> values = new ArrayList<>(keys.size());
+ for (byte[] valueBytes : valueBytesList) {
+ values.add(decodeValue(valueBytes));
+ }
+ return values;
+ }
+
+ List<VALUE> multiGetFromTableWithCodecBuffer(List<KEY> keys)
+ throws RocksDatabaseException, CodecException {
+ final int n = keys.size();
+ final List<CodecBuffer> keyBuffers = new ArrayList<>(n);
+ final List<ByteBuffer> keyByteBuffers = new ArrayList<>(n);
+ final List<CodecBuffer> valueBuffers = new ArrayList<>(n);
+ final List<ByteBuffer> valueByteBuffers = new ArrayList<>(n);
+ final int initialCapacity = bufferCapacity.get();
+ for (KEY key : keys) {
+ final CodecBuffer keyBuffer = keyCodec.toDirectCodecBuffer(key);
+ keyBuffers.add(keyBuffer);
+ keyByteBuffers.add(keyBuffer.asReadOnlyByteBuffer());
+ final CodecBuffer valueBuffer = CodecBuffer.allocateDirect(initialCapacity);
+ valueBuffers.add(valueBuffer);
+ addWritableValueByteBuffer(valueBuffer, initialCapacity, valueByteBuffers);
+ }
+
+ try {
+ final List<VALUE> values = new ArrayList<>(Collections.nCopies(n, null));
+ final List<Integer> retryIndices = new ArrayList<>();
+ final List<Integer> retrySizes = new ArrayList<>();
+ try {
+ applyMultiGetStatuses(rawTable.multiGetSkipCache(keyByteBuffers, valueByteBuffers),
+ valueBuffers, initialCapacity, values, retryIndices, retrySizes);
+ if (!retryIndices.isEmpty()) {
+ retryOversizedMultiGet(keyByteBuffers, retryIndices, retrySizes, values);
+ bufferCapacity.increase(Collections.max(retrySizes));
+ }
+ return values;
+ } finally {
+ IOUtils.closeQuietly(valueBuffers);
+ }
+ } finally {
+ IOUtils.closeQuietly(keyBuffers);
+ }
+ }
+
+ void applyMultiGetStatuses(List<ByteBufferGetStatus> statuses, List<CodecBuffer> valueBuffers,
+ int valueBufferCapacity, List<VALUE> values, List<Integer> retryIndices, List<Integer> retrySizes)
+ throws CodecException {
+ for (int i = 0; i < statuses.size(); i++) {
+ final ByteBufferGetStatus status = statuses.get(i);
+ final CodecBuffer valueBuffer = valueBuffers.get(i);
+ if (status.value == null) {
+ continue;
+ }
+ validateMultiGetStatus(status, null);
+ if (status.requiredSize > valueBufferCapacity) {
+ retryIndices.add(i);
+ retrySizes.add(status.requiredSize);
+ } else {
+ values.set(i, decodeFromMultiGetStatus(status, valueBuffer));
+ }
+ }
+ }
+
+ void retryOversizedMultiGet(List<ByteBuffer> keyByteBuffers, List<Integer> retryIndices,
+ List<Integer> retrySizes, List<VALUE> values) throws RocksDatabaseException, CodecException {
+ final int retryCount = retryIndices.size();
+ final List<ByteBuffer> retryKeyByteBuffers = new ArrayList<>(retryCount);
+ final List<CodecBuffer> retryValueBuffers = new ArrayList<>(retryCount);
+ final List<ByteBuffer> retryValueByteBuffers = new ArrayList<>(retryCount);
+ for (int j = 0; j < retryCount; j++) {
+ final int requiredSize = retrySizes.get(j);
+ retryKeyByteBuffers.add(keyByteBuffers.get(retryIndices.get(j)).duplicate());
+ final CodecBuffer valueBuffer = CodecBuffer.allocateDirect(requiredSize);
+ retryValueBuffers.add(valueBuffer);
+ addWritableValueByteBuffer(valueBuffer, requiredSize, retryValueByteBuffers);
+ }
+
+ try {
+ final List<ByteBufferGetStatus> retryStatuses =
+ rawTable.multiGetSkipCache(retryKeyByteBuffers, retryValueByteBuffers);
+ for (int j = 0; j < retryCount; j++) {
+ final int index = retryIndices.get(j);
+ final int expectedSize = retrySizes.get(j);
+ final ByteBufferGetStatus status = retryStatuses.get(j);
+ final CodecBuffer valueBuffer = retryValueBuffers.get(j);
+ if (status.value == null) {
+ continue;
+ }
+ validateMultiGetStatus(status, expectedSize);
+ values.set(index, decodeFromMultiGetStatus(status, valueBuffer));
+ }
+ } finally {
+ IOUtils.closeQuietly(retryValueBuffers);
+ }
+ }
+
+ static void addWritableValueByteBuffer(CodecBuffer valueBuffer, int capacity,
+ List<ByteBuffer> valueByteBuffers) {
+ valueBuffer.clear();
+ final ByteBuffer out = valueBuffer.asWritableByteBuffer();
+ out.position(0).limit(capacity);
+ valueByteBuffers.add(out);
+ }
+
+ static void validateMultiGetStatus(ByteBufferGetStatus status, Integer expectedSize) {
+ if (status.requiredSize < 0 || status.status.getCode() != org.rocksdb.Status.Code.Ok) {
+ throw new IllegalStateException("status = " + status.status.getCode()
+ + "; required = " + status.requiredSize + " < 0");
Review Comment:
The IllegalStateException message in validateMultiGetStatus is misleading: it always says "< 0" even when the failure is due to a non-OK RocksDB status code. This makes debugging multiGet failures harder.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-16229. Add RocksDB multiGetSkipCache for batched table reads. [ozone]
Posted by "SaketaChalamchala (via GitHub)" <gi...@apache.org>.
SaketaChalamchala commented on code in PR #11068:
URL: https://github.com/apache/ozone/pull/11068#discussion_r3927411664
##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/RDBTable.java:
##########
@@ -122,10 +124,32 @@ public byte[] get(byte[] key) throws RocksDatabaseException {
return db.get(family, key);
}
+ @Override
+ public List<byte[]> multiGetSkipCache(List<byte[]> keys) throws RocksDatabaseException {
+ if (keys == null || keys.isEmpty()) {
+ return Collections.emptyList();
+ }
+ for (byte[] ignored : keys) {
+ rdbMetrics.incNumDBKeyGets();
+ }
+ return db.multiGet(family, keys);
+ }
+
Integer get(ByteBuffer key, ByteBuffer outValue) throws RocksDatabaseException {
return db.get(family, key, outValue);
}
+ List<ByteBufferGetStatus> multiGetSkipCache(List<ByteBuffer> keys, List<ByteBuffer> values)
+ throws RocksDatabaseException {
+ if (keys == null || keys.isEmpty()) {
+ return Collections.emptyList();
+ }
+ for (ByteBuffer ignored : keys) {
+ rdbMetrics.incNumDBKeyGets();
Review Comment:
Created [HDDS-16380](https://issues.apache.org/jira/browse/HDDS-16380)
--
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.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-16229. Add RocksDB multiGetSkipCache for batched table reads. [ozone]
Posted by "jojochuang (via GitHub)" <gi...@apache.org>.
jojochuang commented on PR #11068:
URL: https://github.com/apache/ozone/pull/11068#issuecomment-5431120657
Two performance notes on the CodecBuffer `multiGetFromTable` path:
### 1. Upfront allocation: `n × initial` direct memory
The first pass allocates a direct value buffer for **every** key in the batch before calling RocksDB:
```java
final int initialCapacity = bufferCapacity.get(); // default 4 KB
for (KEY key : keys) {
...
final CodecBuffer valueBuffer = CodecBuffer.allocateDirect(initialCapacity);
...
}
```
For a batch of size `n`, that is roughly **`n × initialCapacity`** of direct memory reserved up front (plus `n` direct key buffers), even when most values are small or missing. With large batches this can create noticeable direct-memory pressure and GC churn when buffers are closed in the finally blocks.
The byte[] path avoids this — RocksDB allocates only the actual value sizes. Worth calling out in the PR description if callers may pass large key lists (e.g. OM/SCM metadata scans). A possible follow-up would be lazy/per-key allocation or capping batch size at the call site.
### 2. `bufferCapacity` hint not updated after oversize retry
Single-key reads adapt the shared hint when a value exceeds the current capacity:
```java
// getFromTable() — after resize/retry loop
bufferCapacity.increase(required);
```
`multiGetFromTable` reads `bufferCapacity.get()` for the initial pass but **never calls `bufferCapacity.increase()`** when keys are retried with `requiredSize`. On a table whose values are consistently larger than the default (4 KB), every `multiGetSkipCache` call will pay the two-pass penalty (first pass discovers oversize, second pass re-allocates per oversize key), while single `get()` calls on the same table would have warmed the hint.
Consider updating the hint after the retry pass, e.g. `bufferCapacity.increase(Collections.max(retrySizes))`, so subsequent multiGets on that table start with a better initial capacity. Low priority for correctness, but it would align multi-get behavior with the existing single-get adaptive sizing.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-16229. Add RocksDB multiGetSkipCache for batched table reads. [ozone]
Posted by "yandrey321 (via GitHub)" <gi...@apache.org>.
yandrey321 commented on code in PR #11068:
URL: https://github.com/apache/ozone/pull/11068#discussion_r3898450297
##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/RDBTable.java:
##########
@@ -122,10 +124,32 @@ public byte[] get(byte[] key) throws RocksDatabaseException {
return db.get(family, key);
}
+ @Override
+ public List<byte[]> multiGetSkipCache(List<byte[]> keys) throws RocksDatabaseException {
+ if (keys == null || keys.isEmpty()) {
+ return Collections.emptyList();
+ }
+ for (byte[] ignored : keys) {
+ rdbMetrics.incNumDBKeyGets();
+ }
+ return db.multiGet(family, keys);
+ }
+
Integer get(ByteBuffer key, ByteBuffer outValue) throws RocksDatabaseException {
return db.get(family, key, outValue);
}
+ List<ByteBufferGetStatus> multiGetSkipCache(List<ByteBuffer> keys, List<ByteBuffer> values)
+ throws RocksDatabaseException {
+ if (keys == null || keys.isEmpty()) {
+ return Collections.emptyList();
+ }
+ for (ByteBuffer ignored : keys) {
+ rdbMetrics.incNumDBKeyGets();
Review Comment:
please open JIRA for this improvement
--
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.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-16229. Add RocksDB multiGetSkipCache for batched table reads. [ozone]
Posted by "yandrey321 (via GitHub)" <gi...@apache.org>.
yandrey321 commented on code in PR #11068:
URL: https://github.com/apache/ozone/pull/11068#discussion_r3884731024
##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/RDBTable.java:
##########
@@ -122,10 +124,32 @@ public byte[] get(byte[] key) throws RocksDatabaseException {
return db.get(family, key);
}
+ @Override
+ public List<byte[]> multiGetSkipCache(List<byte[]> keys) throws RocksDatabaseException {
+ if (keys == null || keys.isEmpty()) {
+ return Collections.emptyList();
+ }
+ for (byte[] ignored : keys) {
+ rdbMetrics.incNumDBKeyGets();
+ }
+ return db.multiGet(family, keys);
+ }
+
Integer get(ByteBuffer key, ByteBuffer outValue) throws RocksDatabaseException {
return db.get(family, key, outValue);
}
+ List<ByteBufferGetStatus> multiGetSkipCache(List<ByteBuffer> keys, List<ByteBuffer> values)
+ throws RocksDatabaseException {
+ if (keys == null || keys.isEmpty()) {
+ return Collections.emptyList();
+ }
+ for (ByteBuffer ignored : keys) {
+ rdbMetrics.incNumDBKeyGets();
Review Comment:
why not just inc to the keys.size()?
--
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.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-16229. Add RocksDB multiGetSkipCache for batched table reads. [ozone]
Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi commented on code in PR #11068:
URL: https://github.com/apache/ozone/pull/11068#discussion_r3819028356
##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/TypedTable.java:
##########
@@ -228,6 +229,24 @@ public VALUE getSkipCache(KEY key) throws RocksDatabaseException, CodecException
return getFromTable(key);
}
+ @Override
+ public List<VALUE> multiGetSkipCache(List<KEY> keys)
Review Comment:
Please implement CodeBuffer based multiGets as well otherwise the GC pressure on memory would be bad
https://github.com/facebook/rocksdb/blob/805ac7c887bbebd7e6124bbc7e8d0803aeed8f07/java/src/main/java/org/rocksdb/RocksDB.java#L2435-L2460
You would need to implement a similar function like
https://github.com/apache/ozone/blob/9fa7fffb4c6245caaad305655a73501bfb5f6bb6/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/TypedTable.java#L318-L355
to ensure the value which didn't fit in the first multiget should be called again with a resized CodecBuffer size.
So the first call should do a multiGet with all CodecBuffer keys and based on the status list returned we need to call multiget again for the ones which didn't fit in the initial fit and collate the list.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-16229. Add RocksDB multiGetSkipCache for batched table reads. [ozone]
Posted by "SaketaChalamchala (via GitHub)" <gi...@apache.org>.
SaketaChalamchala commented on code in PR #11068:
URL: https://github.com/apache/ozone/pull/11068#discussion_r3898359142
##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/RDBTable.java:
##########
@@ -122,10 +124,32 @@ public byte[] get(byte[] key) throws RocksDatabaseException {
return db.get(family, key);
}
+ @Override
+ public List<byte[]> multiGetSkipCache(List<byte[]> keys) throws RocksDatabaseException {
+ if (keys == null || keys.isEmpty()) {
+ return Collections.emptyList();
+ }
+ for (byte[] ignored : keys) {
+ rdbMetrics.incNumDBKeyGets();
+ }
+ return db.multiGet(family, keys);
+ }
+
Integer get(ByteBuffer key, ByteBuffer outValue) throws RocksDatabaseException {
return db.get(family, key, outValue);
}
+ List<ByteBufferGetStatus> multiGetSkipCache(List<ByteBuffer> keys, List<ByteBuffer> values)
+ throws RocksDatabaseException {
+ if (keys == null || keys.isEmpty()) {
+ return Collections.emptyList();
+ }
+ for (ByteBuffer ignored : keys) {
+ rdbMetrics.incNumDBKeyGets();
Review Comment:
Need to add the `incNumDBKeyGets(n)` getter to the `RDBMetrics`. Maybe we can address this in a follow up PR for metric improvements.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-16229. Add RocksDB multiGetSkipCache for batched table reads. [ozone]
Posted by "jojochuang (via GitHub)" <gi...@apache.org>.
jojochuang commented on code in PR #11068:
URL: https://github.com/apache/ozone/pull/11068#discussion_r3899732144
##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/RDBTable.java:
##########
@@ -122,10 +124,32 @@ public byte[] get(byte[] key) throws RocksDatabaseException {
return db.get(family, key);
}
+ @Override
+ public List<byte[]> multiGetSkipCache(List<byte[]> keys) throws RocksDatabaseException {
+ if (keys == null || keys.isEmpty()) {
+ return Collections.emptyList();
+ }
+ for (byte[] ignored : keys) {
+ rdbMetrics.incNumDBKeyGets();
+ }
+ return db.multiGet(family, keys);
+ }
+
Integer get(ByteBuffer key, ByteBuffer outValue) throws RocksDatabaseException {
return db.get(family, key, outValue);
}
+ List<ByteBufferGetStatus> multiGetSkipCache(List<ByteBuffer> keys, List<ByteBuffer> values)
+ throws RocksDatabaseException {
+ if (keys == null || keys.isEmpty()) {
+ return Collections.emptyList();
+ }
+ for (ByteBuffer ignored : keys) {
+ rdbMetrics.incNumDBKeyGets();
Review Comment:
yeah makes sense. Though comparing to RocksDB I/O this is probably negligible impact.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-16229. Add RocksDB multiGetSkipCache for batched table reads. [ozone]
Posted by "SaketaChalamchala (via GitHub)" <gi...@apache.org>.
SaketaChalamchala commented on PR #11068:
URL: https://github.com/apache/ozone/pull/11068#issuecomment-5532634777
Fixed the log. Added missing relavent tests and rebased the PR.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-16229. Add RocksDB multiGetSkipCache for batched table reads. [ozone]
Posted by "jojochuang (via GitHub)" <gi...@apache.org>.
jojochuang commented on PR #11068:
URL: https://github.com/apache/ozone/pull/11068#issuecomment-5431117970
@SaketaChalamchala — I took a pass at refactoring `TypedTable.multiGetFromTable()` to address the duplicated status handling between the first pass and the oversize retry loop.
**Branch:** https://github.com/jojochuang/ozone/tree/HDDS-16229-refactor
**Commit:** https://github.com/jojochuang/ozone/commit/0504b5bc8e4
### What changed
- Split the CodecBuffer vs byte[] paths into `multiGetFromTableWithCodecBuffer()` / `multiGetFromTableWithByteArray()`.
- Extracted shared helpers:
- `applyMultiGetStatuses()` — first pass (decode or queue retry when `requiredSize > capacity`)
- `retryOversizedMultiGet()` — second batch for oversize keys
- `validateMultiGetStatus()`, `decodeFromMultiGetStatus()`, `addWritableValueByteBuffer()`
- Retry keys use `keyByteBuffer.duplicate()` so we don't reuse a buffer whose position may have moved.
Behavior should be unchanged; this is a readability/dedup refactor only.
### Validation
- `TestRDBTable`
- `TestRDBTableStore#multiGetSkipCache`
- `TestRDBTableStore#multiGetSkipCacheWithTypedTable`
- checkstyle on `:hdds-server-framework`
Feel free to cherry-pick or adapt if useful.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-16229. Add RocksDB multiGetSkipCache for batched table reads. [ozone]
Posted by "yandrey321 (via GitHub)" <gi...@apache.org>.
yandrey321 commented on code in PR #11068:
URL: https://github.com/apache/ozone/pull/11068#discussion_r3884729965
##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/RDBTable.java:
##########
@@ -122,10 +124,32 @@ public byte[] get(byte[] key) throws RocksDatabaseException {
return db.get(family, key);
}
+ @Override
+ public List<byte[]> multiGetSkipCache(List<byte[]> keys) throws RocksDatabaseException {
+ if (keys == null || keys.isEmpty()) {
+ return Collections.emptyList();
+ }
+ for (byte[] ignored : keys) {
Review Comment:
why not just inc to the keys.size()?
--
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.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-16229. Add RocksDB multiGetSkipCache for batched table reads. [ozone]
Posted by "jojochuang (via GitHub)" <gi...@apache.org>.
jojochuang merged PR #11068:
URL: https://github.com/apache/ozone/pull/11068
--
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.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org