You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "kerneltime (via GitHub)" <gi...@apache.org> on 2024/01/17 23:44:36 UTC
[PR] HDDS-10154: isKeyPresentInTable should use the constructor with prefix [ozone]
kerneltime opened a new pull request, #6022:
URL: https://github.com/apache/ozone/pull/6022
## What changes were proposed in this pull request?
isKeyPresentInTable should use the constructor with prefix.
Please describe your PR in detail:
The table iterator implemented over rocksDB can seek to the prefix if provided at the time to construction. This can avoid wasteful seekToFirst behavior that is part of the construction of the iterator.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-10154
## How was this patch tested?
Existing 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-10154. isKeyPresentInTable should use the constructor with prefix [ozone]
Posted by "kerneltime (via GitHub)" <gi...@apache.org>.
kerneltime commented on PR #6022:
URL: https://github.com/apache/ozone/pull/6022#issuecomment-1899416522
> @kerneltime can you explain the difference in the two options? Looking at the code it seems both options end up calling `RDBStoreAbstractIterator#seek0` soon after the iterator is created. Based on the [RocksDB API](https://github.com/facebook/rocksdb/wiki/Prefix-Seek#transition-to-the-new-usage) it does not look like the underlying implementation does anything different in these two cases.
When the constructor is called without prefix
```
RDBStoreByteArrayIterator(ManagedRocksIterator iterator,
RDBTable table, byte[] prefix) {
super(iterator, table,
prefix == null ? null : Arrays.copyOf(prefix, prefix.length));
seekToFirst();
}
```
seekToFirst() does
```
public final void seekToFirst() {
if (prefix == null) {
rocksDBIterator.get().seekToFirst();
} else {
seek0(prefix);
}
setCurrentEntry();
}
```
which in turn calls
```
@Override
public void seekToFirst() {
assert (isOwningHandle());
seekToFirst0(nativeHandle_);
}
```
--
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-10154. isKeyPresentInTable should use the constructor with prefix [ozone]
Posted by "kerneltime (via GitHub)" <gi...@apache.org>.
kerneltime commented on PR #6022:
URL: https://github.com/apache/ozone/pull/6022#issuecomment-1899421218
> @kerneltime can you explain the difference in the two options? Looking at the code it seems both options end up calling `RDBStoreAbstractIterator#seek0` soon after the iterator is created. Based on the [RocksDB API](https://github.com/facebook/rocksdb/wiki/Prefix-Seek#transition-to-the-new-usage) it does not look like the underlying implementation does anything different in these two cases.
This behavior was first fixed for https://issues.apache.org/jira/browse/HDDS-8289. There needs to be a clean up of TableIterators to behave the same way when in memory vs via RocksDB. My initial attempt to clean up this code did not go as cleanly as this works only when the table type is RocksDB. I am continuing to investigate how to make the behavior the same (always initialize the iterator with prefix when prefix is available).
--
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-10154. isKeyPresentInTable should use the constructor with prefix [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai merged PR #6022:
URL: https://github.com/apache/ozone/pull/6022
--
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-10154. isKeyPresentInTable should use the constructor with prefix [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #6022:
URL: https://github.com/apache/ozone/pull/6022#issuecomment-1901118132
Thanks @kerneltime for the patch, @duongkame, @errose28 for the review.
--
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-10154. isKeyPresentInTable should use the constructor with prefix [ozone]
Posted by "errose28 (via GitHub)" <gi...@apache.org>.
errose28 commented on PR #6022:
URL: https://github.com/apache/ozone/pull/6022#issuecomment-1899279827
@kerneltime can you explain the difference in the two options? Looking at the code it seems both options end up calling `RDBStoreAbstractIterator#seek0` soon after the iterator is created. Based on the [RocksDB API](https://github.com/facebook/rocksdb/wiki/Prefix-Seek#transition-to-the-new-usage) it does not look like the underlying implementation does anything different in these two cases.
--
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