You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "smengcl (via GitHub)" <gi...@apache.org> on 2026/07/06 18:15:53 UTC
[PR] HDDS-14225. [WIP] Upgrade RocksDB from 7.7.3 to 11.1.2 [ozone]
smengcl opened a new pull request, #9813:
URL: https://github.com/apache/ozone/pull/9813
Generated-by: GPT-5.3-Codex
## What changes were proposed in this pull request?
Bump RocksDB version from 7.7.3 to ~~10.4.2~~ 10.10.1. While maintaining compatibility and not breaking anything.
1. Since RocksDB 9, BlockBasedTableOptions.format_version=6 is the default. Files written in format_version 6 cannot be read by RocksDB < 8.6.0. This PR explicitly sets the default to version 5 to be compatible with 7.7.3 in the case where Ozone gets downgraded before it gets finalized after an upgrade. ref: https://github.com/facebook/rocksdb/releases/tag/v9.0.0
2. DO NOT MERGE until v10.9.0 or higher RocksDB JNI is available. Potential forward compatibility bug < v10.9.0: https://github.com/facebook/rocksdb/releases/tag/v10.9.1
> Fix a bug where compaction with range deletion can persist kTypeMaxValid in MANIFEST as file metadata. kTypeMaxValid is not supposed to be persisted and can change as new value types are introduced. This can cause a forward compatibility issue where older versions of RocksDB don't recognize kTypeMaxValid from newer versions. A new placeholder value type kTypeTruncatedRangeDeletionSentinel is also introduced to replace kTypeMaxValid when reading existing SST files' metadata from MANIFEST. This allows us to strengthen some checks to avoid using kTypeMaxValid in the future.
3. Currently in this PR I am using a rocksdbjni 10.10.1 fatjar I built myself and pushed to maven central snapshot. We need to properly publish it (for example, under Apache Ozone account) before we can merge this.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-14225
## How was this patch tested?
- Most RocksDB related tests are fixed.
- Pending full CI.
--
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-14225. [DO NOT MERGE] Upgrade RocksDB from 7.7.3 to 10.10.1 [ozone]
Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on PR #9813:
URL: https://github.com/apache/ozone/pull/9813#issuecomment-4049997520
Good news. rocksdb team just published 10.5.1 jars: https://repo1.maven.org/maven2/org/rocksdb/rocksdbjni/10.5.1/
But we need at least 10.9.0 to avoid the forward compatibility bug mentioned in the description. Need to wait a bit more
--
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-14225. [WIP] Upgrade RocksDB from 7.7.3 to 10.10.1 [ozone]
Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on code in PR #9813:
URL: https://github.com/apache/ozone/pull/9813#discussion_r3533112410
##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/RDBTable.java:
##########
@@ -99,16 +99,16 @@ public boolean isEmpty() throws RocksDatabaseException {
@Override
public boolean isExist(byte[] key) throws RocksDatabaseException {
rdbMetrics.incNumDBKeyMayExistChecks();
- final Supplier<byte[]> holder = db.keyMayExist(family, key);
- if (holder == null) {
- return false; // definitely not exists
- }
- final byte[] value = holder.get();
- if (value != null) {
- return true; // definitely exists
+ final Supplier<byte[]> valueSupplier = db.keyMayExist(family, key);
+ if (valueSupplier != null) {
+ final byte[] value = valueSupplier.get();
+ if (value != null) {
+ return true; // definitely exists
+ }
}
- // inconclusive: the key may or may not exist
+ // keyMayExist can be a false-negative in some RocksDB setups (for example,
+ // snapshot/checkpoint DBs). Verify via point-get to preserve correctness.
final boolean exists = get(key) != null;
Review Comment:
Sorry about the confusion. The comment has been reworded:
```
// keyMayExist could not return the value; confirm via point-get.
```
--
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-14225. [WIP] Upgrade RocksDB from 7.7.3 to 10.10.1 [ozone]
Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on code in PR #9813:
URL: https://github.com/apache/ozone/pull/9813#discussion_r3533107461
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java:
##########
@@ -966,9 +966,22 @@ private <T> CheckedFunction<KeyManager, T, IOException> getPreviousSnapshotOzone
OmBucketInfo bucketInfo, long objectId, String currentKeyPath,
Function<KeyManager, Table<String, T>> table) throws IOException {
String renameKey = metadataManager.getRenameKey(bucketInfo.getVolumeName(), bucketInfo.getBucketName(), objectId);
- String renamedKey = metadataManager.getSnapshotRenamedTable().getIfExist(renameKey);
- return (previousSnapshotKM) -> table.apply(previousSnapshotKM).get(
- renamedKey != null ? renamedKey : currentKeyPath);
+ Table<String, String> renamedTable = metadataManager.getSnapshotRenamedTable();
+ String renamedKey = renamedTable.getIfExist(renameKey);
Review Comment:
interim getSkipCache change reverted. not applicable anymore
--
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-14225. [WIP] Upgrade RocksDB from 7.7.3 to 10.10.1 [ozone]
Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on code in PR #9813:
URL: https://github.com/apache/ozone/pull/9813#discussion_r3533105843
##########
hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedBloomFilter.java:
##########
@@ -28,6 +28,16 @@
public class ManagedBloomFilter extends BloomFilter {
private final UncheckedAutoCloseable leakTracker = track(this);
+ @Override
Review Comment:
This suppresses spotbugs `EQ_DOESNT_OVERRIDE_EQUALS`
--
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-14225. [WIP] Upgrade RocksDB from 7.7.3 to 10.10.1 [ozone]
Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on code in PR #9813:
URL: https://github.com/apache/ozone/pull/9813#discussion_r3533104366
##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/RDBTable.java:
##########
@@ -141,15 +141,15 @@ public byte[] getSkipCache(byte[] bytes) throws RocksDatabaseException {
@Override
public byte[] getIfExist(byte[] key) throws RocksDatabaseException {
Review Comment:
Yes it does
--
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-14225. Upgrade RocksDB from 7.7.3 to 10.10.1 [ozone]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #9813:
URL: https://github.com/apache/ozone/pull/9813#discussion_r3533973082
##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/RocksDatabase.java:
##########
@@ -888,6 +891,12 @@ public void deleteFilesNotMatchingPrefix(TablePrefixInfo prefixInfo) throws Rock
boolean isKeyWithPrefixPresent = RocksDiffUtils.isKeyWithPrefixPresent(
prefixForColumnFamily, firstDbKey, lastDbKey);
if (!isKeyWithPrefixPresent) {
+ ColumnFamilyHandle handle = getColumnFamilyHandle(sstFileColumnFamily);
+ if (handle == null) {
+ LOG.warn("Skipping sst file deletion for {}: no handle found for column family {}",
+ liveFileMetaData.fileName(), sstFileColumnFamily);
+ continue;
Review Comment:
The level-check comment above this block still refers to the removed RocksDB#deleteFile behavior, but the implementation now deletes via deleteFilesInRanges (ManagedRocksDB.deleteSstFileRange). Please update the comment to describe the current rationale; otherwise it’s misleading for future maintenance.
--
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-14225. [WIP] Upgrade RocksDB from 7.7.3 to 10.10.1 [ozone]
Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on code in PR #9813:
URL: https://github.com/apache/ozone/pull/9813#discussion_r3533085526
##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/RDBTable.java:
##########
@@ -141,15 +141,15 @@ public byte[] getSkipCache(byte[] bytes) throws RocksDatabaseException {
@Override
public byte[] getIfExist(byte[] key) throws RocksDatabaseException {
rdbMetrics.incNumDBKeyGetIfExistChecks();
- final Supplier<byte[]> value = db.keyMayExist(family, key);
Review Comment:
done
--
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-14225. Upgrade RocksDB from 7.7.3 to 10.10.1 [ozone]
Posted by "jojochuang (via GitHub)" <gi...@apache.org>.
jojochuang commented on code in PR #9813:
URL: https://github.com/apache/ozone/pull/9813#discussion_r3539214249
##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/DBProfile.java:
##########
@@ -87,6 +87,7 @@ public ManagedBlockBasedTableConfig getBlockBasedTableConfig() {
ManagedBlockBasedTableConfig config = new ManagedBlockBasedTableConfig();
config.setBlockCache(new ManagedLRUCache(blockCacheSize))
.setBlockSize(blockSize)
+ .setFormatVersion(ManagedBlockBasedTableConfig.FORMAT_VERSION)
Review Comment:
up until now, the sst file in the rocksdb used by Ozone does not define format version.
Future versions of Ozone, if they bump the format version, can roll back to as early as this version of Ozone (2.3.0?), but they can't roll back to before 2.3.0. Otherwise the sst generated by future Ozone version may not be compatible.
##########
hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedBloomFilter.java:
##########
@@ -28,6 +28,20 @@
public class ManagedBloomFilter extends BloomFilter {
private final UncheckedAutoCloseable leakTracker = track(this);
+ // Delegate to satisfy SpotBugs EQ_DOESNT_OVERRIDE_EQUALS: BloomFilter defines
Review Comment:
another approach is annotate the class to supress the warning.
```
@SuppressFBWarnings(
value = "EQ_DOESNT_OVERRIDE_EQUALS",
justification = "leakTracker is resource-tracking state and not part of logical identity"
)
```
##########
hadoop-hdds/rocksdb-checkpoint-differ/src/main/java/org/apache/hadoop/hdds/utils/db/RDBSstFileWriter.java:
##########
@@ -36,8 +37,13 @@ public class RDBSstFileWriter implements Closeable {
private AtomicLong keyCounter;
private ManagedOptions emptyOption = new ManagedOptions();
private final ManagedEnvOptions emptyEnvOptions = new ManagedEnvOptions();
+ private final ManagedBlockBasedTableConfig tableConfig = new ManagedBlockBasedTableConfig();
public RDBSstFileWriter(File externalFile) throws RocksDatabaseException {
+ // Pin the SST format version so files written here (e.g. snapshot defrag
+ // ingest) stay readable if Ozone is downgraded before finalization.
+ tableConfig.setFormatVersion(ManagedBlockBasedTableConfig.FORMAT_VERSION);
Review Comment:
Need an upgrade test later. We'll need it for 2.3.0 release anyway.
Actually, we're missing upgrade tests for 2.1 and 2.2 too https://github.com/apache/ozone/tree/master/hadoop-ozone/dist/src/main/compose/upgrade/upgrades/non-rolling-upgrade/callbacks
--
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-14225. [WIP] Upgrade RocksDB from 7.7.3 to 10.10.1 [ozone]
Posted by "ptlrs (via GitHub)" <gi...@apache.org>.
ptlrs commented on PR #9813:
URL: https://github.com/apache/ozone/pull/9813#issuecomment-4282801560
>Since RocksDB 9, BlockBasedTableOptions.format_version=6 is the default. Files written in format_version 6 cannot be read by RocksDB < 8.6.0. This PR explicitly sets the default to version 5 to be compatible with 7.7.3 in the case where Ozone gets downgraded before it gets finalized after an upgrade. ref: https://github.com/facebook/rocksdb/releases/tag/v9.0.0
Can we explore what it would take to support the newer file formats?
What are the challenges with the following and other potential solutions:
- For a fresh installation, we could use the newer format by default.
- For an upgrade scenario, we could consider re-ingesting the SST files.
--
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-14225. [WIP] Upgrade RocksDB from 7.7.3 to 10.10.1 [ozone]
Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on PR #9813:
URL: https://github.com/apache/ozone/pull/9813#issuecomment-4284435025
> > Since RocksDB 9, BlockBasedTableOptions.format_version=6 is the default. Files written in format_version 6 cannot be read by RocksDB < 8.6.0. This PR explicitly sets the default to version 5 to be compatible with 7.7.3 in the case where Ozone gets downgraded before it gets finalized after an upgrade. ref: [facebook/rocksdb@`v9.0.0` (release)](https://github.com/facebook/rocksdb/releases/tag/v9.0.0)
>
> Can we explore what it would take to support the newer file formats? What are the challenges with the following and other potential solutions:
>
> * For a fresh installation, we could use the newer format by default.
> * For an upgrade scenario, we could consider re-ingesting the SST files.
@ptlrs We surely can add a post finalization step to reingest the SST files, but there is no official tool iirc to do such in-place reingestion so basically it is up to us to write the logic to read from old SSTs and write to new ones.
Or, just allow new SST files to be written in format_version=6 post finalization. Old SSTs should get compacted into new format_version=6 SSTs eventually.
I'd prefer the latter.
--
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-14225. [WIP] Upgrade RocksDB from 7.7.3 to 10.10.1 [ozone]
Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on code in PR #9813:
URL: https://github.com/apache/ozone/pull/9813#discussion_r3533135177
##########
hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedDBOptions.java:
##########
@@ -34,7 +34,6 @@ public class ManagedDBOptions extends DBOptions {
private final UncheckedAutoCloseable leakTracker = track(this);
private final AtomicReference<Logger> loggerRef = new AtomicReference<>();
- @Override
Review Comment:
Actually, JNI in v10 no longer has setLogger(logger). So this is overload now, not override. Changed in:
https://github.com/facebook/rocksdb/commit/4835c11cce4d#diff-1536c1746bb06ab2e37be82324b437afc6b0ee159394d0d2c804532515f5a70aL215-L220
Compared with v7.7.3:
https://github.com/facebook/rocksdb/blob/v7.7.3/java/src/main/java/org/rocksdb/DBOptions.java#L219-L224
--
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-14225. [WIP] Upgrade RocksDB from 7.7.3 to 10.10.1 [ozone]
Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on code in PR #9813:
URL: https://github.com/apache/ozone/pull/9813#discussion_r3533195375
##########
hadoop-hdds/rocks-native/src/main/patches/rocks-native.patch:
##########
@@ -380,9 +381,9 @@ index 000000000..5ba8a82ee
+
+ rep_->file_.reset(new RandomAccessFileReader(std::move(file), file_path));
+
-+ FilePrefetchBuffer prefetch_buffer(
-+ 0 /* readahead_size */, 0 /* max_readahead_size */, true /* enable */,
-+ false /* track_min_offset */);
++ FilePrefetchBuffer prefetch_buffer(ReadaheadParams(),
++ !fopts.use_mmap_reads /* enable */,
++ false /* track_min_offset */);
+ if (s.ok()) {
Review Comment:
`raw_sst_file_reader.cc` is right inside this patch file.
I believe the header you are trying to find is `file/file_prefetch_buffer.h`. The constructor has been changed in https://github.com/facebook/rocksdb/commit/5cb2d09d4762
Thus `raw_sst_file_reader.cc` must follow the changes to `table/sst_file_dumper.cc`:
https://github.com/facebook/rocksdb/commit/5cb2d09d4762#diff-a3d4e6aa344de24bb85c472a34af1e00b9cde125fbfb0afe994ca539328f953d
https://github.com/facebook/rocksdb/commit/ed46981bea38#diff-a3d4e6aa344de24bb85c472a34af1e00b9cde125fbfb0afe994ca539328f953d
--
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-14225. Upgrade RocksDB from 7.7.3 to 10.10.1 [ozone]
Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on PR #9813:
URL: https://github.com/apache/ozone/pull/9813#issuecomment-4910537376
Merged. Thanks @adoroszlai @ptlrs @jojochuang for the reviews.
--
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-14225. [WIP] Upgrade RocksDB from 7.7.3 to 10.10.1 [ozone]
Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on code in PR #9813:
URL: https://github.com/apache/ozone/pull/9813#discussion_r3134283699
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/filter/ReclaimableRenameEntryFilter.java:
##########
@@ -83,6 +83,11 @@ private final boolean isRenameEntryReclaimable(Table.KeyValue<String, String> re
if (previousTable != null) {
String prevDbKey = renameEntry.getValue();
WithObjectID withObjectID = previousTable.getIfExist(prevDbKey);
+ if (withObjectID == null) {
Review Comment:
Yes. But adapatations have already been made in RDBTable. This chunk can be removed I think. Let me double check.
--
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-14225. [WIP] Upgrade RocksDB from 7.7.3 to 10.10.1 [ozone]
Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on code in PR #9813:
URL: https://github.com/apache/ozone/pull/9813#discussion_r3108491674
##########
pom.xml:
##########
@@ -203,7 +204,9 @@
<reflections.version>0.10.2</reflections.version>
<reload4j.version>1.2.26</reload4j.version>
<restrict-imports.enforcer.version>3.0.0</restrict-imports.enforcer.version>
- <rocksdb.version>7.7.3</rocksdb.version>
+ <rocksdb.source.version>10.10.1</rocksdb.source.version>
+ <rocksdb.version>10.10.1</rocksdb.version>
+ <rocksdbjni.groupId>org.rocksdb</rocksdbjni.groupId>
Review Comment:
This was for cleanly using non-official rocksdbjni like the one I built myself earlier. This is not useful anymore since RocksDB Java build caught up.
--
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-14225. Upgrade RocksDB from 7.7.3 to 10.10.1 [ozone]
Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl merged PR #9813:
URL: https://github.com/apache/ozone/pull/9813
--
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-14225. Upgrade RocksDB from 7.7.3 to 10.10.1 [ozone]
Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on code in PR #9813:
URL: https://github.com/apache/ozone/pull/9813#discussion_r3540091674
##########
hadoop-hdds/rocksdb-checkpoint-differ/src/main/java/org/apache/hadoop/hdds/utils/db/RDBSstFileWriter.java:
##########
@@ -36,8 +37,13 @@ public class RDBSstFileWriter implements Closeable {
private AtomicLong keyCounter;
private ManagedOptions emptyOption = new ManagedOptions();
private final ManagedEnvOptions emptyEnvOptions = new ManagedEnvOptions();
+ private final ManagedBlockBasedTableConfig tableConfig = new ManagedBlockBasedTableConfig();
public RDBSstFileWriter(File externalFile) throws RocksDatabaseException {
+ // Pin the SST format version so files written here (e.g. snapshot defrag
+ // ingest) stay readable if Ozone is downgraded before finalization.
+ tableConfig.setFormatVersion(ManagedBlockBasedTableConfig.FORMAT_VERSION);
Review Comment:
Let's file JIRAs for those.
--
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-14225. [DO NOT MERGE] Upgrade RocksDB from 7.7.3 to 10.10.1 [ozone]
Posted by "jojochuang (via GitHub)" <gi...@apache.org>.
jojochuang commented on code in PR #9813:
URL: https://github.com/apache/ozone/pull/9813#discussion_r3017456381
##########
pom.xml:
##########
@@ -2570,6 +2430,17 @@
<id>apache.snapshots.https</id>
<url>https://repository.apache.org/content/repositories/snapshots</url>
</repository>
+ <repository>
+ <releases>
+ <enabled>false</enabled>
+ </releases>
+ <snapshots>
+ <enabled>true</enabled>
+ <updatePolicy>always</updatePolicy>
+ </snapshots>
+ <id>central.sonatype.snapshots</id>
+ <url>https://central.sonatype.com/repository/maven-snapshots/</url>
+ </repository>
Review Comment:
no longer needed?
##########
pom.xml:
##########
@@ -203,7 +204,9 @@
<reflections.version>0.10.2</reflections.version>
<reload4j.version>1.2.26</reload4j.version>
<restrict-imports.enforcer.version>3.0.0</restrict-imports.enforcer.version>
- <rocksdb.version>7.7.3</rocksdb.version>
+ <rocksdb.source.version>10.10.1</rocksdb.source.version>
+ <rocksdb.version>10.10.1</rocksdb.version>
+ <rocksdbjni.groupId>org.rocksdb</rocksdbjni.groupId>
Review Comment:
just curious why do we need to make the dependency group a variable here.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/filter/ReclaimableRenameEntryFilter.java:
##########
@@ -83,6 +83,11 @@ private final boolean isRenameEntryReclaimable(Table.KeyValue<String, String> re
if (previousTable != null) {
String prevDbKey = renameEntry.getValue();
WithObjectID withObjectID = previousTable.getIfExist(prevDbKey);
+ if (withObjectID == null) {
Review Comment:
is this a change of behavior in rocksdb?
##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/DBProfile.java:
##########
@@ -87,6 +87,7 @@ public ManagedBlockBasedTableConfig getBlockBasedTableConfig() {
ManagedBlockBasedTableConfig config = new ManagedBlockBasedTableConfig();
config.setBlockCache(new ManagedLRUCache(blockCacheSize))
.setBlockSize(blockSize)
+ .setFormatVersion(BLOCK_BASED_TABLE_FORMAT_VERSION)
Review Comment:
adding format version field is backward compatible.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java:
##########
@@ -966,9 +966,22 @@ private <T> CheckedFunction<KeyManager, T, IOException> getPreviousSnapshotOzone
OmBucketInfo bucketInfo, long objectId, String currentKeyPath,
Function<KeyManager, Table<String, T>> table) throws IOException {
String renameKey = metadataManager.getRenameKey(bucketInfo.getVolumeName(), bucketInfo.getBucketName(), objectId);
- String renamedKey = metadataManager.getSnapshotRenamedTable().getIfExist(renameKey);
- return (previousSnapshotKM) -> table.apply(previousSnapshotKM).get(
- renamedKey != null ? renamedKey : currentKeyPath);
+ Table<String, String> renamedTable = metadataManager.getSnapshotRenamedTable();
+ String renamedKey = renamedTable.getIfExist(renameKey);
Review Comment:
if getIfExist() is definitive, we wouldn't need to make this change?
##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/RDBTable.java:
##########
@@ -141,15 +141,15 @@ public byte[] getSkipCache(byte[] bytes) throws RocksDatabaseException {
@Override
public byte[] getIfExist(byte[] key) throws RocksDatabaseException {
rdbMetrics.incNumDBKeyGetIfExistChecks();
- final Supplier<byte[]> value = db.keyMayExist(family, key);
Review Comment:
@smengcl can you put the above keyMayExist() behavior change in the PR description.
The goal is to ensure the Ozone API does not change semantics after RocksDB update.
##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/RDBTable.java:
##########
@@ -141,15 +141,15 @@ public byte[] getSkipCache(byte[] bytes) throws RocksDatabaseException {
@Override
public byte[] getIfExist(byte[] key) throws RocksDatabaseException {
rdbMetrics.incNumDBKeyGetIfExistChecks();
- final Supplier<byte[]> value = db.keyMayExist(family, key);
Review Comment:
from commit history:
https://github.com/apache/ozone/pull/9813/changes/24dd41512e7342337e01cc421af6e1e676cba45f
Fix snapshot existence checks after RocksDB 9.10.0 keyMayExist behavior change
RocksDB 9.10.0 changed DB::KeyMayExist behavior semantics to follow its function comment. Ozone snapshot code paths were treating keyMayExist/getIfExist misses as definitive, which could misclassify existing keys in snapshot-related flows under RocksDB >= 9.10.0 and cause test failures.
Treat keyMayExist/getIfExist as hints and fall back to point reads before deciding not-found:
- RDBTable: verify with get()/get(ByteBuffer) on inconclusive keyMayExist
- TypedTable: in codec-buffer isExist path, fallback to full get before returning false
- KeyManagerImpl and ReclaimableRenameEntryFilter: use getSkipCache fallback for snapshot rename lookups
RocksDB changelog for reference:
https://github.com/facebook/rocksdb/releases/tag/v9.10.0
Behavior Changes
DB::KeyMayExist() now follows its function comment, which means value parameter can be null, and it will be set only if value_found is passed in.
##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/RDBTable.java:
##########
@@ -141,15 +141,15 @@ public byte[] getSkipCache(byte[] bytes) throws RocksDatabaseException {
@Override
public byte[] getIfExist(byte[] key) throws RocksDatabaseException {
Review Comment:
does this API provide definitive result?
if not, the javadoc needs to update
##########
hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedBloomFilter.java:
##########
@@ -28,6 +28,16 @@
public class ManagedBloomFilter extends BloomFilter {
private final UncheckedAutoCloseable leakTracker = track(this);
+ @Override
Review Comment:
Looks unnecessary to me
##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/TypedTable.java:
##########
@@ -179,11 +179,17 @@ public boolean isExist(KEY key) throws RocksDatabaseException, CodecException {
} else if (keyCodec.supportCodecBuffer()) {
// keyCodec.supportCodecBuffer() is enough since value is not needed.
try (CodecBuffer inKey = keyCodec.toDirectCodecBuffer(key)) {
- // Use zero capacity buffer since value is not needed.
+ // Try the lightweight "existence only" check first.
try (CodecBuffer outValue = CodecBuffer.getEmptyBuffer()) {
- return getFromTableIfExist(inKey, outValue) != null;
+ if (getFromTableIfExist(inKey, outValue) != null) {
Review Comment:
shouldn't this Ozone API be definitive?
##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/TypedTable.java:
##########
@@ -179,11 +179,17 @@ public boolean isExist(KEY key) throws RocksDatabaseException, CodecException {
} else if (keyCodec.supportCodecBuffer()) {
// keyCodec.supportCodecBuffer() is enough since value is not needed.
try (CodecBuffer inKey = keyCodec.toDirectCodecBuffer(key)) {
- // Use zero capacity buffer since value is not needed.
+ // Try the lightweight "existence only" check first.
try (CodecBuffer outValue = CodecBuffer.getEmptyBuffer()) {
- return getFromTableIfExist(inKey, outValue) != null;
+ if (getFromTableIfExist(inKey, outValue) != null) {
+ return true;
+ }
}
}
+ // keyMayExist/getIfExist can still be false-negative in some RocksDB
+ // configurations (observed with snapshot/checkpoint DBs), so confirm with
+ // a regular point lookup before returning false.
+ return getFromTable(key) != null;
Review Comment:
Is this really necessary? This means an additional lookup.
--
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-14225. [WIP] Upgrade RocksDB from 7.7.3 to 10.10.1 [ozone]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #9813:
URL: https://github.com/apache/ozone/pull/9813#issuecomment-4763797939
This PR has been marked as stale due to 21 days of inactivity. Please comment or remove the stale label to keep it open. Otherwise, it will be automatically closed in 7 days.
--
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-14225. [WIP] Upgrade RocksDB from 7.7.3 to 10.10.1 [ozone]
Posted by "ptlrs (via GitHub)" <gi...@apache.org>.
ptlrs commented on code in PR #9813:
URL: https://github.com/apache/ozone/pull/9813#discussion_r3076165547
##########
pom.xml:
##########
@@ -203,7 +204,9 @@
<reflections.version>0.10.2</reflections.version>
<reload4j.version>1.2.26</reload4j.version>
<restrict-imports.enforcer.version>3.0.0</restrict-imports.enforcer.version>
- <rocksdb.version>7.7.3</rocksdb.version>
+ <rocksdb.source.version>10.10.1</rocksdb.source.version>
+ <rocksdb.version>10.10.1</rocksdb.version>
+ <rocksdbjni.groupId>org.rocksdb</rocksdbjni.groupId>
Review Comment:
I would like to understand this as well. Do we need to parameterize the group name?
--
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-14225. [WIP] Upgrade RocksDB from 7.7.3 to 10.10.1 [ozone]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #9813:
URL: https://github.com/apache/ozone/pull/9813#issuecomment-4585255985
This PR has been marked as stale due to 21 days of inactivity. Please comment or remove the stale label to keep it open. Otherwise, it will be automatically closed in 7 days.
--
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-14225. [WIP] Upgrade RocksDB from 7.7.3 to 10.10.1 [ozone]
Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on code in PR #9813:
URL: https://github.com/apache/ozone/pull/9813#discussion_r3108492934
##########
pom.xml:
##########
@@ -2570,6 +2430,17 @@
<id>apache.snapshots.https</id>
<url>https://repository.apache.org/content/repositories/snapshots</url>
</repository>
+ <repository>
+ <releases>
+ <enabled>false</enabled>
+ </releases>
+ <snapshots>
+ <enabled>true</enabled>
+ <updatePolicy>always</updatePolicy>
+ </snapshots>
+ <id>central.sonatype.snapshots</id>
+ <url>https://central.sonatype.com/repository/maven-snapshots/</url>
+ </repository>
Review Comment:
correct
--
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-14225. [DO NOT MERGE] Upgrade RocksDB from 7.7.3 to 10.10.1 [ozone]
Posted by "jojochuang (via GitHub)" <gi...@apache.org>.
jojochuang commented on PR #9813:
URL: https://github.com/apache/ozone/pull/9813#issuecomment-4165719016
Hi @smengcl thanks a lot for the PR.
My main question is around getIfExist(). If it returns definitive results, a lot of change wouldn't be needed, and it can save a lot of db lookup potentially.
--
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-14225. [WIP] Upgrade RocksDB from 7.7.3 to 10.10.1 [ozone]
Posted by "ptlrs (via GitHub)" <gi...@apache.org>.
ptlrs commented on code in PR #9813:
URL: https://github.com/apache/ozone/pull/9813#discussion_r3075970622
##########
hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedDBOptions.java:
##########
@@ -34,7 +34,6 @@ public class ManagedDBOptions extends DBOptions {
private final UncheckedAutoCloseable leakTracker = track(this);
private final AtomicReference<Logger> loggerRef = new AtomicReference<>();
- @Override
Review Comment:
Do we need to remove this? The RocksDB JNI still has a `setLogger` method available.
##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/RDBTable.java:
##########
@@ -99,16 +99,16 @@ public boolean isEmpty() throws RocksDatabaseException {
@Override
public boolean isExist(byte[] key) throws RocksDatabaseException {
rdbMetrics.incNumDBKeyMayExistChecks();
- final Supplier<byte[]> holder = db.keyMayExist(family, key);
- if (holder == null) {
- return false; // definitely not exists
- }
- final byte[] value = holder.get();
- if (value != null) {
- return true; // definitely exists
+ final Supplier<byte[]> valueSupplier = db.keyMayExist(family, key);
+ if (valueSupplier != null) {
+ final byte[] value = valueSupplier.get();
+ if (value != null) {
+ return true; // definitely exists
+ }
}
- // inconclusive: the key may or may not exist
+ // keyMayExist can be a false-negative in some RocksDB setups (for example,
+ // snapshot/checkpoint DBs). Verify via point-get to preserve correctness.
final boolean exists = get(key) != null;
Review Comment:
I am trying to understand more about this comment or how we can reach this stage.
The `keyMayExist` documentation says that it will never return a false-negative.
Are we trying to say here that a key may not exist in the snapshot/checkpoint DB but may exist in the active DB? In that case the `get(key)` is still running on the same db which would be the snapshot DB instead of the active DB and again not find the key.
> If the key definitely does not exist in the database, then this method
> returns false, otherwise it returns true if the key might exist.
> That is to say that this method is probabilistic and may return false
> positives, but **never a false negative**.
##########
pom.xml:
##########
@@ -203,7 +204,9 @@
<reflections.version>0.10.2</reflections.version>
<reload4j.version>1.2.26</reload4j.version>
<restrict-imports.enforcer.version>3.0.0</restrict-imports.enforcer.version>
- <rocksdb.version>7.7.3</rocksdb.version>
+ <rocksdb.source.version>10.10.1</rocksdb.source.version>
+ <rocksdb.version>10.10.1</rocksdb.version>
+ <rocksdbjni.groupId>org.rocksdb</rocksdbjni.groupId>
Review Comment:
I would like to understand this as well. Do we need to parameterize this?
##########
hadoop-hdds/rocks-native/src/main/patches/rocks-native.patch:
##########
@@ -380,9 +381,9 @@ index 000000000..5ba8a82ee
+
+ rep_->file_.reset(new RandomAccessFileReader(std::move(file), file_path));
+
-+ FilePrefetchBuffer prefetch_buffer(
-+ 0 /* readahead_size */, 0 /* max_readahead_size */, true /* enable */,
-+ false /* track_min_offset */);
++ FilePrefetchBuffer prefetch_buffer(ReadaheadParams(),
++ !fopts.use_mmap_reads /* enable */,
++ false /* track_min_offset */);
+ if (s.ok()) {
Review Comment:
Do we know where the `raw_sst_file_reader` header and implementation files are located?
This [comment](https://github.com/apache/ozone/pull/6182#discussion_r1498510016) mentions that the implementation was originally copied from `sst_file_dumper` but that file doesn't seem to show any signature changes in `10.10.1`.
##########
hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedRocksDB.java:
##########
@@ -112,18 +113,23 @@ public static ManagedRocksDB openWithLatestOptions(
}
/**
- * Delete liveMetaDataFile from rocks db using RocksDB#deleteFile Api.
- * This function makes the RocksDB#deleteFile Api synchronized by waiting
- * for the deletes to happen.
- * @param fileToBeDeleted File to be deleted.
+ * Delete the SST file range from rocks db and wait for file deletion.
+ * @param columnFamilyHandle column family of the target sst file.
+ * @param fileToBeDeleted file metadata to be deleted.
* @throws RocksDatabaseException if the underlying db throws an exception
* or the file is not deleted within a time limit.
*/
- public void deleteFile(LiveFileMetaData fileToBeDeleted) throws RocksDatabaseException {
- String sstFileName = fileToBeDeleted.fileName();
+ public void deleteFile(
+ ColumnFamilyHandle columnFamilyHandle,
+ LiveFileMetaData fileToBeDeleted) throws RocksDatabaseException {
Review Comment:
The `deleteFile` was deprecated in https://github.com/facebook/rocksdb/pull/13284 and removed in https://github.com/facebook/rocksdb/pull/13322.
As a result it should no longer be part of `ManagedRocksDB.java` either.
Any callers of the method should likely be migrated to use the existing `deleteFilesInRanges` method.
--
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-14225. [WIP] Upgrade RocksDB from 7.7.3 to 10.10.1 [ozone]
Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on code in PR #9813:
URL: https://github.com/apache/ozone/pull/9813#discussion_r3533146818
##########
hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedRocksDB.java:
##########
@@ -112,18 +113,23 @@ public static ManagedRocksDB openWithLatestOptions(
}
/**
- * Delete liveMetaDataFile from rocks db using RocksDB#deleteFile Api.
- * This function makes the RocksDB#deleteFile Api synchronized by waiting
- * for the deletes to happen.
- * @param fileToBeDeleted File to be deleted.
+ * Delete the SST file range from rocks db and wait for file deletion.
+ * @param columnFamilyHandle column family of the target sst file.
+ * @param fileToBeDeleted file metadata to be deleted.
* @throws RocksDatabaseException if the underlying db throws an exception
* or the file is not deleted within a time limit.
*/
- public void deleteFile(LiveFileMetaData fileToBeDeleted) throws RocksDatabaseException {
- String sstFileName = fileToBeDeleted.fileName();
+ public void deleteFile(
+ ColumnFamilyHandle columnFamilyHandle,
+ LiveFileMetaData fileToBeDeleted) throws RocksDatabaseException {
Review Comment:
Migrated. Thanks @ptlrs
--
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-14225. Upgrade RocksDB from 7.7.3 to 10.10.1 [ozone]
Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on code in PR #9813:
URL: https://github.com/apache/ozone/pull/9813#discussion_r3540091674
##########
hadoop-hdds/rocksdb-checkpoint-differ/src/main/java/org/apache/hadoop/hdds/utils/db/RDBSstFileWriter.java:
##########
@@ -36,8 +37,13 @@ public class RDBSstFileWriter implements Closeable {
private AtomicLong keyCounter;
private ManagedOptions emptyOption = new ManagedOptions();
private final ManagedEnvOptions emptyEnvOptions = new ManagedEnvOptions();
+ private final ManagedBlockBasedTableConfig tableConfig = new ManagedBlockBasedTableConfig();
public RDBSstFileWriter(File externalFile) throws RocksDatabaseException {
+ // Pin the SST format version so files written here (e.g. snapshot defrag
+ // ingest) stay readable if Ozone is downgraded before finalization.
+ tableConfig.setFormatVersion(ManagedBlockBasedTableConfig.FORMAT_VERSION);
Review Comment:
Let's file JIRAs for those.
Filed https://issues.apache.org/jira/browse/HDDS-15769 for 2.1 and 2.2
--
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-14225. [WIP] Upgrade RocksDB from 7.7.3 to 10.10.1 [ozone]
Posted by "ptlrs (via GitHub)" <gi...@apache.org>.
ptlrs commented on code in PR #9813:
URL: https://github.com/apache/ozone/pull/9813#discussion_r3076159654
##########
hadoop-hdds/rocks-native/src/main/patches/rocks-native.patch:
##########
@@ -380,9 +381,9 @@ index 000000000..5ba8a82ee
+
+ rep_->file_.reset(new RandomAccessFileReader(std::move(file), file_path));
+
-+ FilePrefetchBuffer prefetch_buffer(
-+ 0 /* readahead_size */, 0 /* max_readahead_size */, true /* enable */,
-+ false /* track_min_offset */);
++ FilePrefetchBuffer prefetch_buffer(ReadaheadParams(),
++ !fopts.use_mmap_reads /* enable */,
++ false /* track_min_offset */);
+ if (s.ok()) {
Review Comment:
Do we know where the `raw_sst_file_reader` header and implementation files are located?
This [comment](https://github.com/apache/ozone/pull/6182#discussion_r1498510016) mentions that the implementation was originally copied from `sst_file_dumper` but that file doesn't seem to show any signature changes for `prefetch_buffer` in `10.10.1`.
--
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-14225. [WIP] Upgrade RocksDB from 7.7.3 to 10.10.1 [ozone]
Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on PR #9813:
URL: https://github.com/apache/ozone/pull/9813#issuecomment-4411376091
> Error: Failed to execute goal com.googlecode.maven-download-plugin:download-maven-plugin:1.9.0:wget (rocksdb source download) on project hdds-rocks-native: Download failed with code 404: Not Found -> [Help 1] Error: Error: To see the full stack trace of the errors, re-run Maven with the -e switch. Error: Re-run Maven using the -X switch to enable full debug logging. Error: Error: For more information about the errors and possible solutions, please read the following articles: Error: [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoExecutionException Error: Error: After correcting the problems, you can resume the build with the command Error: mvn -rf :hdds-rocks-native Error: Process completed with exit code 1.
Yup that is because the rocks tools build part uses the same rocksdbjni version.. let me fix that
--
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-14225. Upgrade RocksDB from 7.7.3 to 10.10.1 [ozone]
Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on code in PR #9813:
URL: https://github.com/apache/ozone/pull/9813#discussion_r3540058007
##########
hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedBloomFilter.java:
##########
@@ -28,6 +28,20 @@
public class ManagedBloomFilter extends BloomFilter {
private final UncheckedAutoCloseable leakTracker = track(this);
+ // Delegate to satisfy SpotBugs EQ_DOESNT_OVERRIDE_EQUALS: BloomFilter defines
Review Comment:
`@SuppressFBWarnings` requires `spotbugs-annotations` dep. Ozone code base doesn't have that. I'm not sure it is worth it
--
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-14225. Upgrade RocksDB from 7.7.3 to 10.10.1 [ozone]
Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on code in PR #9813:
URL: https://github.com/apache/ozone/pull/9813#discussion_r3540014653
##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/DBProfile.java:
##########
@@ -87,6 +87,7 @@ public ManagedBlockBasedTableConfig getBlockBasedTableConfig() {
ManagedBlockBasedTableConfig config = new ManagedBlockBasedTableConfig();
config.setBlockCache(new ManagedLRUCache(blockCacheSize))
.setBlockSize(blockSize)
+ .setFormatVersion(ManagedBlockBasedTableConfig.FORMAT_VERSION)
Review Comment:
Yes. RocksDB v10 will be able to read format version 6 SST files.
--
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-14225. Upgrade RocksDB from 7.7.3 to 10.10.1 [ozone]
Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on PR #9813:
URL: https://github.com/apache/ozone/pull/9813#issuecomment-4907476843
I have rebased the PR, address all review comments, did self-review with Opus 4.8 and addressed issues found.
CI has passed in my fork. Marking this ready for 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-14225. Upgrade RocksDB from 7.7.3 to 10.10.1 [ozone]
Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on code in PR #9813:
URL: https://github.com/apache/ozone/pull/9813#discussion_r3534383075
##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/RocksDatabase.java:
##########
@@ -888,6 +891,12 @@ public void deleteFilesNotMatchingPrefix(TablePrefixInfo prefixInfo) throws Rock
boolean isKeyWithPrefixPresent = RocksDiffUtils.isKeyWithPrefixPresent(
prefixForColumnFamily, firstDbKey, lastDbKey);
if (!isKeyWithPrefixPresent) {
+ ColumnFamilyHandle handle = getColumnFamilyHandle(sstFileColumnFamily);
+ if (handle == null) {
+ LOG.warn("Skipping sst file deletion for {}: no handle found for column family {}",
+ liveFileMetaData.fileName(), sstFileColumnFamily);
+ continue;
Review Comment:
it is updated
--
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