-
Notifications
You must be signed in to change notification settings - Fork 588
HDDS-12742. Make RDBStoreAbstractIterator Return Reference-Counted KeyValues #8203
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
30facb9
aab6ebc
5dd7460
e4bf8f7
9ab91c9
abe06ab
d215d4b
85c74b4
7d82a9e
ce6ab81
091e8ca
6bc5c86
1b394c2
7fdced2
f5b633b
1971a96
a33f265
0c3ae4f
fbe213b
71f9c28
5734027
99c508e
f16e1b9
86fe101
cfde81f
f825754
e4155a6
985d612
b98968b
78d33af
ed632a8
58f81cc
3f77cbe
1fecc85
deb0011
346a685
f6b651c
93c7f91
f81c116
7b37a29
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,81 @@ | ||
| /* | ||
| * 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; | ||
|
|
||
| import org.apache.ratis.util.Preconditions; | ||
|
|
||
| /** | ||
| * A utility class for managing an underlying {@link CodecBuffer} with dynamic capacity adjustment | ||
| * based on the requirements of a data source. This class encapsulates operations to allocate, | ||
| * prepare, and release the buffer, as well as retrieve data from a source. | ||
| */ | ||
| public class Buffer { | ||
| private final CodecBuffer.Capacity initialCapacity; | ||
| private final PutToByteBuffer<RuntimeException> source; | ||
| private CodecBuffer buffer; | ||
|
|
||
| public Buffer(CodecBuffer.Capacity initialCapacity, | ||
| PutToByteBuffer<RuntimeException> source) { | ||
| this.initialCapacity = initialCapacity; | ||
| this.source = source; | ||
| } | ||
|
|
||
| public void release() { | ||
| if (buffer != null) { | ||
| buffer.release(); | ||
| } | ||
| } | ||
|
|
||
| public void prepare() { | ||
| if (buffer == null) { | ||
| allocate(); | ||
| } else { | ||
| buffer.clear(); | ||
| } | ||
| } | ||
|
|
||
| public void allocate() { | ||
| if (buffer != null) { | ||
| buffer.release(); | ||
| } | ||
| buffer = CodecBuffer.allocateDirect(-initialCapacity.get()); | ||
| } | ||
|
|
||
| public CodecBuffer getFromDb() { | ||
| for (prepare(); ; allocate()) { | ||
| final Integer required = buffer.putFromSource(source); | ||
| if (required == null) { | ||
| return null; // the source is unavailable | ||
| } else if (required == buffer.readableBytes()) { | ||
| return buffer; // buffer size is big enough | ||
| } | ||
| // buffer size too small, try increasing the capacity. | ||
| if (buffer.setCapacity(required)) { | ||
| buffer.clear(); | ||
| // retry with the new capacity | ||
| final int retried = buffer.putFromSource(source); | ||
| Preconditions.assertSame(required.intValue(), retried, "required"); | ||
| return buffer; | ||
| } | ||
|
|
||
| // failed to increase the capacity | ||
| // increase initial capacity and reallocate it | ||
| initialCapacity.increase(required); | ||
| } | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -226,7 +226,7 @@ public RDBStore build() throws IOException { | |
| return new RDBStore(dbFile, rocksDBOption, statistics, writeOptions, tableConfigs, | ||
| openReadOnly, dbJmxBeanNameName, enableCompactionDag, | ||
| maxDbUpdatesSizeThreshold, createCheckpointDirs, configuration, | ||
| enableRocksDbMetrics); | ||
| enableRocksDbMetrics, rocksDBConfiguration.isThreadSafeIteratorEnabled()); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we should not define configuration as threadsafe or not as we are not exposing SDK to be integrated to other solution. Ozone should work as thread safe in usages, may be some layer is not threadsafe. |
||
| } finally { | ||
| tableConfigs.forEach(TableConfig::close); | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -73,6 +73,7 @@ public class RDBStore implements DBStore { | |
| private final long maxDbUpdatesSizeThreshold; | ||
| private final ManagedDBOptions dbOptions; | ||
| private final ManagedStatistics statistics; | ||
| private final boolean initializeReferenceCountedIterator; | ||
|
|
||
| @SuppressWarnings("parameternumber") | ||
| RDBStore(File dbFile, ManagedDBOptions dbOptions, ManagedStatistics statistics, | ||
|
|
@@ -82,13 +83,14 @@ public class RDBStore implements DBStore { | |
| long maxDbUpdatesSizeThreshold, | ||
| boolean createCheckpointDirs, | ||
| ConfigurationSource configuration, | ||
| boolean enableRocksDBMetrics) | ||
| boolean enableRocksDBMetrics, boolean initializeReferenceCountedIterator) | ||
|
|
||
| throws IOException { | ||
| Preconditions.checkNotNull(dbFile, "DB file location cannot be null"); | ||
| Preconditions.checkNotNull(families); | ||
| Preconditions.checkArgument(!families.isEmpty()); | ||
| this.maxDbUpdatesSizeThreshold = maxDbUpdatesSizeThreshold; | ||
| this.initializeReferenceCountedIterator = initializeReferenceCountedIterator; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this name should be initializeThreadSafeIterator as being the config and used with this context. |
||
| dbLocation = dbFile; | ||
| this.dbOptions = dbOptions; | ||
| this.statistics = statistics; | ||
|
|
@@ -308,7 +310,7 @@ public RDBTable getTable(String name) throws IOException { | |
| if (handle == null) { | ||
| throw new IOException("No such table in this DB. TableName : " + name); | ||
| } | ||
| return new RDBTable(this.db, handle, rdbMetrics); | ||
| return new RDBTable(this.db, handle, rdbMetrics, initializeReferenceCountedIterator); | ||
| } | ||
|
|
||
| @Override | ||
|
|
@@ -321,7 +323,7 @@ public <K, V> TypedTable<K, V> getTable( | |
| public ArrayList<Table> listTables() { | ||
| ArrayList<Table> returnList = new ArrayList<>(); | ||
| for (ColumnFamily family : getColumnFamilies()) { | ||
| returnList.add(new RDBTable(db, family, rdbMetrics)); | ||
| returnList.add(new RDBTable(db, family, rdbMetrics, initializeReferenceCountedIterator)); | ||
| } | ||
| return returnList; | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,6 +20,7 @@ | |
| import java.io.IOException; | ||
| import java.util.NoSuchElementException; | ||
| import java.util.function.Consumer; | ||
| import org.apache.hadoop.hdds.utils.db.iterator.BaseDBTableIterator; | ||
| import org.apache.hadoop.hdds.utils.db.managed.ManagedRocksIterator; | ||
| import org.slf4j.Logger; | ||
| import org.slf4j.LoggerFactory; | ||
|
|
@@ -30,14 +31,14 @@ | |
| * @param <RAW> the raw type. | ||
| */ | ||
| abstract class RDBStoreAbstractIterator<RAW> | ||
| implements TableIterator<RAW, Table.KeyValue<RAW, RAW>> { | ||
| implements BaseDBTableIterator<RAW, RawKeyValue<RAW>> { | ||
|
|
||
| private static final Logger LOG = | ||
| LoggerFactory.getLogger(RDBStoreAbstractIterator.class); | ||
|
|
||
| private final ManagedRocksIterator rocksDBIterator; | ||
| private final RDBTable rocksDBTable; | ||
| private Table.KeyValue<RAW, RAW> currentEntry; | ||
| private RawKeyValue<RAW> currentEntry; | ||
| // This is for schemas that use a fixed-length | ||
| // prefix for each key. | ||
| private final RAW prefix; | ||
|
|
@@ -53,7 +54,7 @@ abstract class RDBStoreAbstractIterator<RAW> | |
| abstract RAW key(); | ||
|
|
||
| /** @return the {@link Table.KeyValue} for the current entry. */ | ||
| abstract Table.KeyValue<RAW, RAW> getKeyValue(); | ||
| abstract RawKeyValue<RAW> getKeyValue(); | ||
|
|
||
| /** Seek to the given key. */ | ||
| abstract void seek0(RAW key); | ||
|
|
@@ -78,7 +79,7 @@ final RAW getPrefix() { | |
|
|
||
| @Override | ||
| public final void forEachRemaining( | ||
| Consumer<? super Table.KeyValue<RAW, RAW>> action) { | ||
| Consumer<? super RawKeyValue<RAW>> action) { | ||
| while (hasNext()) { | ||
| action.accept(next()); | ||
| } | ||
|
|
@@ -99,7 +100,7 @@ public final boolean hasNext() { | |
| } | ||
|
|
||
| @Override | ||
| public final Table.KeyValue<RAW, RAW> next() { | ||
| public final RawKeyValue<RAW> next() { | ||
| setCurrentEntry(); | ||
| if (currentEntry != null) { | ||
| rocksDBIterator.get().next(); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. rocksDbIterator.get().next() can also be moved to setCurrentEntry(), as we are getting next entry, updating hasNext flag together.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. no we cannot do that. What about for the first entry. You shouldn't call next for the first entry in the iterator |
||
|
|
@@ -129,7 +130,7 @@ public final void seekToLast() { | |
| } | ||
|
|
||
| @Override | ||
| public final Table.KeyValue<RAW, RAW> seek(RAW key) { | ||
| public final RawKeyValue<RAW> seek(RAW key) { | ||
| seek0(key); | ||
| setCurrentEntry(); | ||
| return currentEntry; | ||
|
|
@@ -151,4 +152,9 @@ public final void removeFromDB() throws IOException { | |
| public void close() { | ||
| rocksDBIterator.close(); | ||
| } | ||
|
|
||
| @Override | ||
| public boolean isKVCloseable() { | ||
| return false; | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
default implementation can throw exception as not supported, no need to add for all cases.