You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "swamirishi (via GitHub)" <gi...@apache.org> on 2025/12/12 00:00:12 UTC
[PR] HDDS-14159. Have an option to read only Key in ManagedRawSSTFileIterator [ozone]
swamirishi opened a new pull request, #9485:
URL: https://github.com/apache/ozone/pull/9485
## What changes were proposed in this pull request?
Currently for SnapshotDiff and defrag we only rely on Key and never on the value of the sst file. We should make the value read optional to avoid unnecessary memory allocation.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-14159
## How was this patch tested?
Added 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-14159. Have an option to read only Key in ManagedRawSSTFileIterator [ozone]
Posted by "jojochuang (via GitHub)" <gi...@apache.org>.
jojochuang commented on code in PR #9485:
URL: https://github.com/apache/ozone/pull/9485#discussion_r2615690318
##########
hadoop-hdds/rocksdb-checkpoint-differ/src/main/java/org/apache/ozone/rocksdb/util/SstFileSetReader.java:
##########
@@ -131,30 +135,30 @@ public void close() throws UncheckedIOException {
}
public ClosableIterator<String> getKeyStreamWithTombstone(String lowerBound, String upperBound)
- throws RocksDBException {
+ throws RocksDBException, CodecException {
final MultipleSstFileIterator<String> itr = new MultipleSstFileIterator<String>(sstFiles) {
//TODO: [SNAPSHOT] Check if default Options is enough.
private ManagedOptions options;
private ManagedSlice lowerBoundSlice;
private ManagedSlice upperBoundSlice;
@Override
- protected void init() {
+ protected void init() throws CodecException {
this.options = new ManagedOptions();
if (Objects.nonNull(lowerBound)) {
this.lowerBoundSlice = new ManagedSlice(
- StringUtils.string2Bytes(lowerBound));
+ StringCodec.get().toPersistedFormat(lowerBound));
}
if (Objects.nonNull(upperBound)) {
this.upperBoundSlice = new ManagedSlice(
- StringUtils.string2Bytes(upperBound));
+ StringCodec.get().toPersistedFormat(upperBound));
}
}
@Override
protected ClosableIterator<String> getKeyIteratorForFile(String file) {
return new ManagedRawSstFileIterator(file, options, lowerBoundSlice, upperBoundSlice,
- keyValue -> StringUtils.bytes2String(keyValue.getKey()));
+ keyValue -> StringCodec.get().fromPersistedFormat(keyValue.getKey()), KEY_ONLY);
Review Comment:
SstFileSetReader reads just the key.
--
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-14159. Have an option to read only Key in ManagedRawSSTFileIterator [ozone]
Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi merged PR #9485:
URL: https://github.com/apache/ozone/pull/9485
--
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-14159. Have an option to read only Key in ManagedRawSSTFileIterator [ozone]
Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi commented on code in PR #9485:
URL: https://github.com/apache/ozone/pull/9485#discussion_r2615686352
##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/utils/db/IteratorType.java:
##########
@@ -0,0 +1,48 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hdds.utils.db;
+
+/**
+ * The iterator type.
+ */
+public enum IteratorType {
+ /**
+ * Neither read key nor value.
+ */
+ NEITHER,
+ /**
+ * Read key only.
+ */
+ KEY_ONLY,
+ /**
+ * Read value only.
+ */
+ VALUE_ONLY,
+ /**
+ * Read both key and value.
+ */
+ KEY_AND_VALUE;
+
+ public boolean readKey() {
+ return (this.ordinal() & KEY_ONLY.ordinal()) != 0;
+ }
+
+ public boolean readValue() {
+ return (this.ordinal() & VALUE_ONLY.ordinal()) != 0;
+ }
Review Comment:
nice thanks for pointing out. I just moved this code from internal TableIterator. Let me make this change
--
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-14159. Have an option to read only Key in ManagedRawSSTFileIterator [ozone]
Posted by "SaketaChalamchala (via GitHub)" <gi...@apache.org>.
SaketaChalamchala commented on code in PR #9485:
URL: https://github.com/apache/ozone/pull/9485#discussion_r2615496843
##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/utils/db/IteratorType.java:
##########
@@ -0,0 +1,48 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hdds.utils.db;
+
+/**
+ * The iterator type.
+ */
+public enum IteratorType {
+ /**
+ * Neither read key nor value.
+ */
+ NEITHER,
+ /**
+ * Read key only.
+ */
+ KEY_ONLY,
+ /**
+ * Read value only.
+ */
+ VALUE_ONLY,
+ /**
+ * Read both key and value.
+ */
+ KEY_AND_VALUE;
+
+ public boolean readKey() {
+ return (this.ordinal() & KEY_ONLY.ordinal()) != 0;
+ }
+
+ public boolean readValue() {
+ return (this.ordinal() & VALUE_ONLY.ordinal()) != 0;
+ }
Review Comment:
Thanks for the patch @swamirishi
nit: set the bit mask explicitly instead of relying on the ordinal just to avoid any issues if the enum constants are reordered/added in the future.
```suggestion
NEITHER(0),
/**
* Read key only.
*/
KEY_ONLY(1),
/**
* Read value only.
*/
VALUE_ONLY(2),
/**
* Read both key and value.
*/
KEY_AND_VALUE(3);
public final int mask;
IteratorType(int mask) {
this.mask = mask;
}
public boolean readKey() {
return (this.mask & KEY_ONLY.mask) != 0;
}
public boolean readValue() {
return (this.mask & VALUE_ONLY.mask) != 0;
}
```
--
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-14159. Have an option to read only Key in ManagedRawSSTFileIterator [ozone]
Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi commented on PR #9485:
URL: https://github.com/apache/ozone/pull/9485#issuecomment-3648730531
thank you @jojochuang and @SaketaChalamchala for reviewing the the patch
--
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