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 2022/04/13 07:36:56 UTC
[GitHub] [flink-benchmarks] fredia opened a new pull request, #52: [FLINK-27214] Fix rescaling benchmark build failed
fredia opened a new pull request, #52:
URL: https://github.com/apache/flink-benchmarks/pull/52
In `Iteration` level, `prepareStateForOperator()` may be called repeatedly, which causes build failed. So, change the level of `Setup/TearDown` in the rescaling benchmark from `Iteration` to `Invocation`.
--
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@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-benchmarks] Myasuka commented on pull request #52: [FLINK-27214] Fix rescaling benchmark build failed
Posted by GitBox <gi...@apache.org>.
Myasuka commented on PR #52:
URL: https://github.com/apache/flink-benchmarks/pull/52#issuecomment-1098018316
Current problem is because `#rescaleRocksDB` executed multi times without call `#tearDownPerInvocation` after each call.
From my understanding, if we take a look at the generated code by jmh, we can see the logic below:
~~~java
public BenchmarkTaskResult rescaleRocksDB_AverageTime(InfraControl control, ThreadParams threadParams) throws Throwable {
...
control.preSetup();
l_rocksdbstatebackendrescalingbenchmarkexecutor0_0.setUpPerInvocation();
control.announceWarmupReady();
while (control.warmupShouldWait) {
l_rocksdbstatebackendrescalingbenchmarkexecutor0_0.rescaleRocksDB();
res.allOps++;
}
...
}
~~~
If the `#rescaleRocksDB()` only executed once, that would be fine. However, we cannot ensure only once execution for this method within the while loop.
If we change the annotataion to `Level.Invocation`, generated code would be:
~~~java
public BenchmarkTaskResult rescaleRocksDB_AverageTime(InfraControl control, ThreadParams threadParams) throws Throwable {
...
while (control.warmupShouldWait) {
l_rocksdbstatebackendrescalingbenchmarkexecutor0_0.setUpPerInvocation();
l_rocksdbstatebackendrescalingbenchmarkexecutor0_0.rescaleRocksDB();
l_rocksdbstatebackendrescalingbenchmarkexecutor0_0.tearDownPerInvocation();
res.allOps++;
}
...
}
~~~
Even the while loop could be executed multi times, since we call the `setup` and `tearDown` around the benchamrk, there would be no problem.
Since current execution time would be larger than one second, I think we would not hit the warning of inaccurate results.
Thus, I am +1 for merging this fix.
--
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@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-benchmarks] fredia commented on pull request #52: [FLINK-27214] Fix rescaling benchmark build failed
Posted by GitBox <gi...@apache.org>.
fredia commented on PR #52:
URL: https://github.com/apache/flink-benchmarks/pull/52#issuecomment-1097706789
From the JMH [documentation](http://javadox.com/org.openjdk.jmh/jmh-core/1.7/org/openjdk/jmh/annotations/Level.html#Invocation), Invocation level setup/teardowns are problematic.
Although the warning points out that using `Level.Invocation` may lead to inaccurate results, the fastest single iteration in rescaling benchmarks takes more than 1 second, maybe we can ignore the warning?
@pnowojski would you please take a review? And my understanding of `Level.Invocation` may not be accurate, would you please give some advice about this?
--
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@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-benchmarks] Myasuka merged pull request #52: [FLINK-27214] Fix rescaling benchmark build failed
Posted by GitBox <gi...@apache.org>.
Myasuka merged PR #52:
URL: https://github.com/apache/flink-benchmarks/pull/52
--
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@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-benchmarks] Myasuka commented on pull request #52: [FLINK-27214] Fix rescaling benchmark build failed
Posted by GitBox <gi...@apache.org>.
Myasuka commented on PR #52:
URL: https://github.com/apache/flink-benchmarks/pull/52#issuecomment-1099145980
Let's merge this PR first to not break the daily flink benchmarks. We can tune parameters in the future if necessary.
--
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@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-benchmarks] pnowojski commented on pull request #52: [FLINK-27214] Fix rescaling benchmark build failed
Posted by GitBox <gi...@apache.org>.
pnowojski commented on PR #52:
URL: https://github.com/apache/flink-benchmarks/pull/52#issuecomment-1098008280
Sorry, I don't have time at the moment to look deeper into this problem and how to potentially avoid using `Level.Invocation`. If you can not figure out a way to do it, we should either revert the benchmarks or merge this fix as it is.
--
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@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org