You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by GitBox <gi...@apache.org> on 2020/06/08 04:30:49 UTC
[GitHub] [flink] Myasuka opened a new pull request #12514: [FLINK-17800][roksdb] Ensure total order seek to avoid user misuse
Myasuka opened a new pull request #12514:
URL: https://github.com/apache/flink/pull/12514
## What is the purpose of the change
Flink open RocksDB column family options configuration for users. However, user might misuse `OptimizeForPointLookup` which lead to RocksDB timer and map state did not give correct result.
The perfect solution is to bump RocksDB version larger than 6.2.2 (see https://github.com/facebook/rocksdb/issues/6893 ), however, this might be better done in release-1.12 cycle and we use explicitly `new ReadOptions().setTotalOrderSeek(true)` to pass to RocksDB iterators to ensure the seek order.
## Brief change log
- Add method `RocksDBOperationUtils#createTotalOrderSeekReadOptions` to create read options and pass this to places where need a RocksDB iterator.
## Verifying this change
This change added tests and can be verified as follows:
- Added parameter running for `RocksDBStateBackendTest` to verify tests could still get the correct result even user misuse `OptimizeForPointLookup`.
- Added parameter running for `RocksDBRocksStateKeysIteratorTest` to ensure iterator actions could still get the correct result even user misuse `OptimizeForPointLookup`.
## Does this pull request potentially affect one of the following parts:
- Dependencies (does it add or upgrade a dependency): no
- The public API, i.e., is any changed class annotated with `@Public(Evolving)`: no
- The serializers: no
- The runtime per-record code paths (performance sensitive): **should no**
- Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Kubernetes/Yarn/Mesos, ZooKeeper: no
- The S3 file system connector: no
## Documentation
- Does this pull request introduce a new feature? no
- If yes, how is the feature documented? not applicable
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #12514: [FLINK-17800][roksdb] Ensure total order seek to avoid user misuse
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #12514:
URL: https://github.com/apache/flink/pull/12514#issuecomment-640366641
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "ee5c3fbc124705fb074c252345685a8370cace80",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=2897",
"triggerID" : "ee5c3fbc124705fb074c252345685a8370cace80",
"triggerType" : "PUSH"
}, {
"hash" : "890188e1935bfcd1a8772c6816f8531a2e771520",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=2904",
"triggerID" : "890188e1935bfcd1a8772c6816f8531a2e771520",
"triggerType" : "PUSH"
}, {
"hash" : "ece2f96e252ebc56009a9f063d7fc04e65d58f7e",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=3022",
"triggerID" : "ece2f96e252ebc56009a9f063d7fc04e65d58f7e",
"triggerType" : "PUSH"
}, {
"hash" : "4fd80ba26b8a7f4899459074157979fc3d03e7f2",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=3204",
"triggerID" : "4fd80ba26b8a7f4899459074157979fc3d03e7f2",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* ece2f96e252ebc56009a9f063d7fc04e65d58f7e Azure: [SUCCESS](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=3022)
* 4fd80ba26b8a7f4899459074157979fc3d03e7f2 Azure: [PENDING](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=3204)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] Myasuka commented on pull request #12514: [FLINK-17800][roksdb] Ensure total order seek to avoid user misuse
Posted by GitBox <gi...@apache.org>.
Myasuka commented on pull request #12514:
URL: https://github.com/apache/flink/pull/12514#issuecomment-643028882
@flinkbot run azure
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #12514: [FLINK-17800][roksdb] Ensure total order seek to avoid user misuse
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #12514:
URL: https://github.com/apache/flink/pull/12514#issuecomment-640366641
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "ee5c3fbc124705fb074c252345685a8370cace80",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=2897",
"triggerID" : "ee5c3fbc124705fb074c252345685a8370cace80",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* ee5c3fbc124705fb074c252345685a8370cace80 Azure: [FAILURE](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=2897)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] carp84 commented on pull request #12514: [FLINK-17800][roksdb] Ensure total order seek to avoid user misuse
Posted by GitBox <gi...@apache.org>.
carp84 commented on pull request #12514:
URL: https://github.com/apache/flink/pull/12514#issuecomment-643363267
> Forwarding my comments from the #12529 PR (the 1.11.0 port of this PR)
>
> I would suggest that we avoid "parameterized" tests for this. I think this is not a configuration option that we need to cross test all with all unit tests. This adds a huge amount of test runtime.
>
> Instead I would add tests that are very targeted, for the behavior that is affected by this (range iterators, like in MapState and in Timers).
@StephanEwen It seems the latest changes here have followed the suggestions and extracted the test cases into a new `RocksDBStateMisuseOptionTest` class. Would you mind to take another look? Thanks.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] carp84 commented on pull request #12514: [FLINK-17800][roksdb] Ensure total order seek to avoid user misuse
Posted by GitBox <gi...@apache.org>.
carp84 commented on pull request #12514:
URL: https://github.com/apache/flink/pull/12514#issuecomment-641072766
Let's also test the performance and make sure the newly added `ReadOptions.setTotalOrderSeek(true)` logic won't cause any regression for none `optimizeForPointLookup` 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] StephanEwen commented on pull request #12514: [FLINK-17800][roksdb] Ensure total order seek to avoid user misuse
Posted by GitBox <gi...@apache.org>.
StephanEwen commented on pull request #12514:
URL: https://github.com/apache/flink/pull/12514#issuecomment-643677068
All in all, looks good to me.
One thing that surprised me a bit was adding the `ReadOptions` and `WriteOptions` to the `PredefinedOptions`. I always thought that the `PredefinedOptions` are a "shortcut" for users to configure common options.
These read options and write options are not configurable options in the end, but "must-have" options that users cannot change (similar to the RocksDB option "create if not exists") so I am a bit confused about them being added to a class that managed user-defined options.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #12514: [FLINK-17800][roksdb] Ensure total order seek to avoid user misuse
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #12514:
URL: https://github.com/apache/flink/pull/12514#issuecomment-640366641
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "ee5c3fbc124705fb074c252345685a8370cace80",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=2897",
"triggerID" : "ee5c3fbc124705fb074c252345685a8370cace80",
"triggerType" : "PUSH"
}, {
"hash" : "890188e1935bfcd1a8772c6816f8531a2e771520",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "890188e1935bfcd1a8772c6816f8531a2e771520",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* ee5c3fbc124705fb074c252345685a8370cace80 Azure: [FAILURE](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=2897)
* 890188e1935bfcd1a8772c6816f8531a2e771520 UNKNOWN
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] StephanEwen commented on pull request #12514: [FLINK-17800][roksdb] Ensure total order seek to avoid user misuse
Posted by GitBox <gi...@apache.org>.
StephanEwen commented on pull request #12514:
URL: https://github.com/apache/flink/pull/12514#issuecomment-644164408
I think having these in the `RocksDBOptionsFactory` is fine. I am confused a bit about the `PedefinedOptions` because you cannot override anything there (it is a fixed et of enums).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] carp84 commented on pull request #12514: [FLINK-17800][roksdb] Ensure total order seek to avoid user misuse
Posted by GitBox <gi...@apache.org>.
carp84 commented on pull request #12514:
URL: https://github.com/apache/flink/pull/12514#issuecomment-643880827
The main reason of making `ReadOptions` configurable through `RocksDBOptionsFactory` is to provide a convenient way to revert the `setTotalOrderSeek(true)` setting and prevent possible (unexpected) performance regression under real-world workload, although we have try our best to safeguard it through micro-benchmark.
OTOH, we actually find there're more setters in `ReadOptions` and `WriteOptions` than what we set as default:
* ReadOptions: `setFillCache`, `setManaged`, `setPinData`, `setReadTier`, **`setTotalOrderSeek`**, `setVerifyChecksums`, etc.
* WriteOptions: **`setDisableWAL`**, `setIgnoreMissingColumnFamilies`, `setNoSlowdown`, `setSync`, etc.
However, I could also understand if we choose to hide these detailed options from our users and believe our micro-benchmark result that `setTotalOrderSeek(true)` won't cause any regression in Flink use case. So just to clarify my thoughts and look forward to your opinion @StephanEwen. Thanks.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] carp84 commented on pull request #12514: [FLINK-17800][roksdb] Ensure total order seek to avoid user misuse
Posted by GitBox <gi...@apache.org>.
carp84 commented on pull request #12514:
URL: https://github.com/apache/flink/pull/12514#issuecomment-643083157
From the [benchmark result](https://s.apache.org/jvs7u), everything looks good except for `listAddAll`. @Myasuka Could you run the `listAddAll` benchmark locally and see whether the regression is stable?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] carp84 commented on a change in pull request #12514: [FLINK-17800][roksdb] Ensure total order seek to avoid user misuse
Posted by GitBox <gi...@apache.org>.
carp84 commented on a change in pull request #12514:
URL: https://github.com/apache/flink/pull/12514#discussion_r437131212
##########
File path: flink-state-backends/flink-statebackend-rocksdb/src/test/java/org/apache/flink/contrib/streaming/state/RocksDBResource.java
##########
@@ -101,7 +101,7 @@ public ColumnFamilyOptions createColumnOptions(ColumnFamilyOptions currentOption
LOG.error("Close previous ColumnOptions's instance failed.", e);
}
- return PredefinedOptions.FLASH_SSD_OPTIMIZED.createColumnOptions(handlesToClose);
+ return PredefinedOptions.FLASH_SSD_OPTIMIZED.createColumnOptions(handlesToClose).optimizeForPointLookup(40960);
Review comment:
We should cover both the `optimizeForPointLookup` and normal case here, instead of `optimizeForPointLookup` only
##########
File path: flink-state-backends/flink-statebackend-rocksdb/src/test/java/org/apache/flink/contrib/streaming/state/RocksDBResource.java
##########
@@ -155,7 +155,7 @@ protected void before() throws Throwable {
PredefinedOptions.DEFAULT.createColumnOptions(handlesToClose), handlesToClose);
this.writeOptions = new WriteOptions();
this.writeOptions.disableWAL();
- this.readOptions = new ReadOptions();
+ this.readOptions = RocksDBOperationUtils.createTotalOrderSeekReadOptions();
Review comment:
Ditto.
##########
File path: flink-state-backends/flink-statebackend-rocksdb/src/test/java/org/apache/flink/contrib/streaming/state/RocksDBStateBackendTest.java
##########
@@ -235,7 +262,9 @@ public Object answer(InvocationOnMock invocationOnMock) throws Throwable {
@Test
public void testCorrectMergeOperatorSet() throws Exception {
prepareRocksDB();
- final ColumnFamilyOptions columnFamilyOptions = spy(new ColumnFamilyOptions());
+ final ColumnFamilyOptions columnFamilyOptions = userMisuseOptimizeForPointLookup ?
+ spy(optimizePointLookupOption(new ColumnFamilyOptions())) :
+ spy(new ColumnFamilyOptions());
Review comment:
These `spy` are required by the later `Mockito.verify`. Since this issue is a blocker and the mockito tests has been existing for a while, let's keep it as is here, but we should find some time to try best to remove these mockito tests later.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot commented on pull request #12514: [FLINK-17800][roksdb] Ensure total order seek to avoid user misuse
Posted by GitBox <gi...@apache.org>.
flinkbot commented on pull request #12514:
URL: https://github.com/apache/flink/pull/12514#issuecomment-640360680
Thanks a lot for your contribution to the Apache Flink project. I'm the @flinkbot. I help the community
to review your pull request. We will use this comment to track the progress of the review.
## Automated Checks
Last check on commit ee5c3fbc124705fb074c252345685a8370cace80 (Mon Jun 08 04:33:54 UTC 2020)
**Warnings:**
* No documentation files were touched! Remember to keep the Flink docs up to date!
<sub>Mention the bot in a comment to re-run the automated checks.</sub>
## Review Progress
* ❓ 1. The [description] looks good.
* ❓ 2. There is [consensus] that the contribution should go into to Flink.
* ❓ 3. Needs [attention] from.
* ❓ 4. The change fits into the overall [architecture].
* ❓ 5. Overall code [quality] is good.
Please see the [Pull Request Review Guide](https://flink.apache.org/contributing/reviewing-prs.html) for a full explanation of the review process.<details>
The Bot is tracking the review progress through labels. Labels are applied according to the order of the review items. For consensus, approval by a Flink committer of PMC member is required <summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot approve description` to approve one or more aspects (aspects: `description`, `consensus`, `architecture` and `quality`)
- `@flinkbot approve all` to approve all aspects
- `@flinkbot approve-until architecture` to approve everything until `architecture`
- `@flinkbot attention @username1 [@username2 ..]` to require somebody's attention
- `@flinkbot disapprove architecture` to remove an approval you gave earlier
</details>
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot commented on pull request #12514: [FLINK-17800][roksdb] Ensure total order seek to avoid user misuse
Posted by GitBox <gi...@apache.org>.
flinkbot commented on pull request #12514:
URL: https://github.com/apache/flink/pull/12514#issuecomment-640366641
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "ee5c3fbc124705fb074c252345685a8370cace80",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "ee5c3fbc124705fb074c252345685a8370cace80",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* ee5c3fbc124705fb074c252345685a8370cace80 UNKNOWN
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] carp84 edited a comment on pull request #12514: [FLINK-17800][roksdb] Ensure total order seek to avoid user misuse
Posted by GitBox <gi...@apache.org>.
carp84 edited a comment on pull request #12514:
URL: https://github.com/apache/flink/pull/12514#issuecomment-643880827
The main reason of making `ReadOptions` configurable through `RocksDBOptionsFactory` is to provide a convenient way to revert the `setTotalOrderSeek(true)` setting and prevent possible (unexpected) performance regression under real-world workload, although we have tried our best to safeguard it through micro-benchmark.
OTOH, we actually find there're more setters in `ReadOptions` and `WriteOptions` than what we set as default:
* ReadOptions: `setFillCache`, `setManaged`, `setPinData`, `setReadTier`, **`setTotalOrderSeek`**, `setVerifyChecksums`, etc.
* WriteOptions: **`setDisableWAL`**, `setIgnoreMissingColumnFamilies`, `setNoSlowdown`, `setSync`, etc.
However, I could also understand if we choose to hide these detailed options from our users and believe our micro-benchmark result that `setTotalOrderSeek(true)` won't cause any regression in Flink use case. So just to clarify my thoughts and look forward to your opinion @StephanEwen. Thanks.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #12514: [FLINK-17800][roksdb] Ensure total order seek to avoid user misuse
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #12514:
URL: https://github.com/apache/flink/pull/12514#issuecomment-640366641
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "ee5c3fbc124705fb074c252345685a8370cace80",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=2897",
"triggerID" : "ee5c3fbc124705fb074c252345685a8370cace80",
"triggerType" : "PUSH"
}, {
"hash" : "890188e1935bfcd1a8772c6816f8531a2e771520",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=2904",
"triggerID" : "890188e1935bfcd1a8772c6816f8531a2e771520",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 890188e1935bfcd1a8772c6816f8531a2e771520 Azure: [FAILURE](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=2904)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #12514: [FLINK-17800][roksdb] Ensure total order seek to avoid user misuse
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #12514:
URL: https://github.com/apache/flink/pull/12514#issuecomment-640366641
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "ee5c3fbc124705fb074c252345685a8370cace80",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=2897",
"triggerID" : "ee5c3fbc124705fb074c252345685a8370cace80",
"triggerType" : "PUSH"
}, {
"hash" : "890188e1935bfcd1a8772c6816f8531a2e771520",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=2904",
"triggerID" : "890188e1935bfcd1a8772c6816f8531a2e771520",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* ee5c3fbc124705fb074c252345685a8370cace80 Azure: [FAILURE](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=2897)
* 890188e1935bfcd1a8772c6816f8531a2e771520 Azure: [PENDING](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=2904)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] Myasuka commented on pull request #12514: [FLINK-17800][roksdb] Ensure total order seek to avoid user misuse
Posted by GitBox <gi...@apache.org>.
Myasuka commented on pull request #12514:
URL: https://github.com/apache/flink/pull/12514#issuecomment-643204059
I run `listAddAll` benchmark locally and the result turns to be no difference for twice run:
| Operation | original | setTotalOrderSeek true |
| ---- | ---- | ---- |
| org.apache.flink.state.benchmark.ListStateBenchmark.listAddAll | 278.836437 ops/ms |279.395449 ops/ms |
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] carp84 edited a comment on pull request #12514: [FLINK-17800][roksdb] Ensure total order seek to avoid user misuse
Posted by GitBox <gi...@apache.org>.
carp84 edited a comment on pull request #12514:
URL: https://github.com/apache/flink/pull/12514#issuecomment-643083157
From the [benchmark result](https://s.apache.org/jvs7u), everything looks good except for `listAddAll` (~10% regression). @Myasuka Could you run the `listAddAll` benchmark locally and see whether the regression is stable?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] StephanEwen commented on pull request #12514: [FLINK-17800][roksdb] Ensure total order seek to avoid user misuse
Posted by GitBox <gi...@apache.org>.
StephanEwen commented on pull request #12514:
URL: https://github.com/apache/flink/pull/12514#issuecomment-643354823
Forwarding my comments from the #12529 PR (the 1.11.0 port of this PR)
I would suggest that we avoid "parameterized" tests for this. I think this is not a configuration option that we need to cross test all with all unit tests. This adds a huge amount of test runtime.
Instead I would add tests that are very targeted, for the behavior that is affected by this (range iterators, like in MapState and in Timers).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] carp84 closed pull request #12514: [FLINK-17800][roksdb] Ensure total order seek to avoid user misuse
Posted by GitBox <gi...@apache.org>.
carp84 closed pull request #12514:
URL: https://github.com/apache/flink/pull/12514
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] carp84 commented on pull request #12514: [FLINK-17800][roksdb] Ensure total order seek to avoid user misuse
Posted by GitBox <gi...@apache.org>.
carp84 commented on pull request #12514:
URL: https://github.com/apache/flink/pull/12514#issuecomment-642701467
Thanks @Myasuka for the updates, the latest changes LGTM. I've triggered a micro-benchmark run on our Jenkins, will give my +1 if there's no performance regression.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #12514: [FLINK-17800][roksdb] Ensure total order seek to avoid user misuse
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #12514:
URL: https://github.com/apache/flink/pull/12514#issuecomment-640366641
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #12514: [FLINK-17800][roksdb] Ensure total order seek to avoid user misuse
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #12514:
URL: https://github.com/apache/flink/pull/12514#issuecomment-640366641
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "ee5c3fbc124705fb074c252345685a8370cace80",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=2897",
"triggerID" : "ee5c3fbc124705fb074c252345685a8370cace80",
"triggerType" : "PUSH"
}, {
"hash" : "890188e1935bfcd1a8772c6816f8531a2e771520",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=2904",
"triggerID" : "890188e1935bfcd1a8772c6816f8531a2e771520",
"triggerType" : "PUSH"
}, {
"hash" : "ece2f96e252ebc56009a9f063d7fc04e65d58f7e",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=3022",
"triggerID" : "ece2f96e252ebc56009a9f063d7fc04e65d58f7e",
"triggerType" : "PUSH"
}, {
"hash" : "4fd80ba26b8a7f4899459074157979fc3d03e7f2",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=3204",
"triggerID" : "4fd80ba26b8a7f4899459074157979fc3d03e7f2",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 4fd80ba26b8a7f4899459074157979fc3d03e7f2 Azure: [FAILURE](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=3204)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #12514: [FLINK-17800][roksdb] Ensure total order seek to avoid user misuse
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #12514:
URL: https://github.com/apache/flink/pull/12514#issuecomment-640366641
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "ee5c3fbc124705fb074c252345685a8370cace80",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=2897",
"triggerID" : "ee5c3fbc124705fb074c252345685a8370cace80",
"triggerType" : "PUSH"
}, {
"hash" : "890188e1935bfcd1a8772c6816f8531a2e771520",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=2904",
"triggerID" : "890188e1935bfcd1a8772c6816f8531a2e771520",
"triggerType" : "PUSH"
}, {
"hash" : "ece2f96e252ebc56009a9f063d7fc04e65d58f7e",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=3022",
"triggerID" : "ece2f96e252ebc56009a9f063d7fc04e65d58f7e",
"triggerType" : "PUSH"
}, {
"hash" : "4fd80ba26b8a7f4899459074157979fc3d03e7f2",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "4fd80ba26b8a7f4899459074157979fc3d03e7f2",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* ece2f96e252ebc56009a9f063d7fc04e65d58f7e Azure: [SUCCESS](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=3022)
* 4fd80ba26b8a7f4899459074157979fc3d03e7f2 UNKNOWN
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] carp84 commented on pull request #12514: [FLINK-17800][roksdb] Ensure total order seek to avoid user misuse
Posted by GitBox <gi...@apache.org>.
carp84 commented on pull request #12514:
URL: https://github.com/apache/flink/pull/12514#issuecomment-642703796
PS. The latest pre-commit check failed on e2e part at `Kubernetes test` due to some connection problem, thus is irrelative to the changes here. It would be nice if we could get a green one with the latest changes, though.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org