Skip to content

Commit ee74231

Browse files
authored
[FLINK-23789][rocksdb] Remove unnecessary setTotalOrderForSeek for Rocks iterator
This fix apache#16834.
1 parent d57d9f8 commit ee74231

File tree

6 files changed

+11
-29
lines changed

6 files changed

+11
-29
lines changed

flink-state-backends/flink-statebackend-rocksdb/src/main/java/org/apache/flink/contrib/streaming/state/RocksDBIncrementalCheckpointUtils.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,7 @@ private static void deleteRange(
127127
throws RocksDBException {
128128

129129
for (ColumnFamilyHandle columnFamilyHandle : columnFamilyHandles) {
130-
try (ReadOptions readOptions = RocksDBOperationUtils.createTotalOrderSeekReadOptions();
130+
try (ReadOptions readOptions = new ReadOptions();
131131
RocksIteratorWrapper iteratorWrapper =
132132
RocksDBOperationUtils.getRocksIterator(
133133
db, columnFamilyHandle, readOptions);

flink-state-backends/flink-statebackend-rocksdb/src/main/java/org/apache/flink/contrib/streaming/state/RocksDBOperationUtils.java

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -104,16 +104,6 @@ public static RocksIteratorWrapper getRocksIterator(
104104
return new RocksIteratorWrapper(db.newIterator(columnFamilyHandle, readOptions));
105105
}
106106

107-
/**
108-
* Create a total order read option to avoid user misuse, see FLINK-17800 for more details.
109-
*
110-
* <p>Note, remember to close the generated {@link ReadOptions} when dispose.
111-
*/
112-
// TODO We would remove this method once we bump RocksDB version larger than 6.2.2.
113-
public static ReadOptions createTotalOrderSeekReadOptions() {
114-
return new ReadOptions().setTotalOrderSeek(true);
115-
}
116-
117107
public static void registerKvStateInformation(
118108
Map<String, RocksDBKeyedStateBackend.RocksDbKvStateInfo> kvStateInformation,
119109
RocksDBNativeMetricMonitor nativeMetricMonitor,

flink-state-backends/flink-statebackend-rocksdb/src/main/java/org/apache/flink/contrib/streaming/state/RocksDBResourceContainer.java

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -183,9 +183,7 @@ public WriteOptions getWriteOptions() {
183183

184184
/** Gets the RocksDB {@link ReadOptions} to be used for read operations. */
185185
public ReadOptions getReadOptions() {
186-
// We ensure total order seek by default to prevent user misuse, see FLINK-17800 for more
187-
// details
188-
ReadOptions opt = RocksDBOperationUtils.createTotalOrderSeekReadOptions();
186+
ReadOptions opt = new ReadOptions();
189187
handlesToClose.add(opt);
190188

191189
// add user-defined options factory, if specified

flink-state-backends/flink-statebackend-rocksdb/src/main/java/org/apache/flink/contrib/streaming/state/restore/RocksDBIncrementalRestoreOperation.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -422,7 +422,7 @@ private RestoredDBInstance(
422422
this.columnFamilyHandles = columnFamilyHandles;
423423
this.columnFamilyDescriptors = columnFamilyDescriptors;
424424
this.stateMetaInfoSnapshots = stateMetaInfoSnapshots;
425-
this.readOptions = RocksDBOperationUtils.createTotalOrderSeekReadOptions();
425+
this.readOptions = new ReadOptions();
426426
}
427427

428428
@Override

flink-state-backends/flink-statebackend-rocksdb/src/test/java/org/apache/flink/contrib/streaming/state/RocksDBResource.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,7 @@ protected void before() throws Throwable {
165165
handlesToClose);
166166
this.writeOptions = new WriteOptions();
167167
this.writeOptions.disableWAL();
168-
this.readOptions = RocksDBOperationUtils.createTotalOrderSeekReadOptions();
168+
this.readOptions = new ReadOptions();
169169
this.columnFamilyHandles = new ArrayList<>(1);
170170
this.rocksDB =
171171
RocksDB.open(
Lines changed: 7 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -52,27 +52,24 @@
5252
import static org.junit.Assert.assertTrue;
5353

5454
/**
55-
* Tests to cover cases that even user misuse some options, RocksDB state-backend could still work
56-
* as expected or give explicit feedback.
55+
* Tests to cover cases that if user choose options previously prone to misuse, embedded RocksDB
56+
* state-backend could still work as expected or give explicit feedback.
5757
*
5858
* <p>RocksDB state-backend has some internal operations based on RocksDB's APIs which is
5959
* transparent for users. However, user could still configure options via {@link
6060
* RocksDBOptionsFactory}, and might lead some operations could not get expected result, e.g.
6161
* FLINK-17800
6262
*/
63-
public class RocksDBStateMisuseOptionTest {
63+
public class RocksDBStateOptionTest {
6464

6565
@Rule public final TemporaryFolder tempFolder = new TemporaryFolder();
6666

6767
/**
68-
* Tests to cover case when user misuse optimizeForPointLookup with iterator interfaces on map
68+
* Tests to cover case when user choose optimizeForPointLookup with iterator interfaces on map
6969
* state.
70-
*
71-
* <p>The option {@link ColumnFamilyOptions#optimizeForPointLookup(long)} would lead to
72-
* iterator.seek with prefix bytes invalid.
7370
*/
7471
@Test
75-
public void testMisuseOptimizePointLookupWithMapState() throws Exception {
72+
public void testUseOptimizePointLookupWithMapState() throws Exception {
7673
RocksDBStateBackend rocksDBStateBackend = createStateBackendWithOptimizePointLookup();
7774
RocksDBKeyedStateBackend<Integer> keyedStateBackend =
7875
createKeyedStateBackend(
@@ -111,14 +108,11 @@ public void testMisuseOptimizePointLookupWithMapState() throws Exception {
111108
}
112109

113110
/**
114-
* Tests to cover case when user misuse optimizeForPointLookup with peek operations on priority
111+
* Tests to cover case when user choose optimizeForPointLookup with peek operations on priority
115112
* queue.
116-
*
117-
* <p>The option {@link ColumnFamilyOptions#optimizeForPointLookup(long)} would lead to
118-
* iterator.seek with prefix bytes invalid.
119113
*/
120114
@Test
121-
public void testMisuseOptimizePointLookupWithPriorityQueue() throws IOException {
115+
public void testUseOptimizePointLookupWithPriorityQueue() throws IOException {
122116
RocksDBStateBackend rocksDBStateBackend = createStateBackendWithOptimizePointLookup();
123117
RocksDBKeyedStateBackend<Integer> keyedStateBackend =
124118
createKeyedStateBackend(

0 commit comments

Comments
 (0)