Github user StefanRRichter commented on a diff in the pull request:
https://github.com/apache/flink/pull/5582#discussion_r192076636
--- Diff:
flink-state-backends/flink-statebackend-rocksdb/src/main/java/org/apache/flink/contrib/streaming/state/RocksDBIncrementalCheckpointUtils.java
Github user StefanRRichter commented on a diff in the pull request:
https://github.com/apache/flink/pull/5582#discussion_r192076060
--- Diff:
flink-state-backends/flink-statebackend-rocksdb/src/main/java/org/apache/flink/contrib/streaming/state/RocksDBIncrementalCheckpointUtils.java
Github user StefanRRichter commented on a diff in the pull request:
https://github.com/apache/flink/pull/5582#discussion_r192075532
--- Diff:
flink-state-backends/flink-statebackend-rocksdb/src/main/java/org/apache/flink/contrib/streaming/state/RocksDBKeyedStateBackend.java
Github user StefanRRichter commented on a diff in the pull request:
https://github.com/apache/flink/pull/5582#discussion_r192074489
--- Diff:
flink-state-backends/flink-statebackend-rocksdb/src/main/java/org/apache/flink/contrib/streaming/state/RocksDBKeyedStateBackend.java
Github user StefanRRichter commented on a diff in the pull request:
https://github.com/apache/flink/pull/5582#discussion_r192072966
--- Diff:
flink-state-backends/flink-statebackend-rocksdb/src/main/java/org/apache/flink/contrib/streaming/state/RocksDBKeyedStateBackend.java
Github user StefanRRichter commented on a diff in the pull request:
https://github.com/apache/flink/pull/5582#discussion_r192072170
--- Diff:
flink-state-backends/flink-statebackend-rocksdb/src/main/java/org/apache/flink/contrib/streaming/state/RocksDBIncrementalCheckpointUtils.java
Github user StefanRRichter commented on a diff in the pull request:
https://github.com/apache/flink/pull/5582#discussion_r192071512
--- Diff:
flink-state-backends/flink-statebackend-rocksdb/src/main/java/org/apache/flink/contrib/streaming/state/RocksDBIncrementalCheckpointUtils.java
Github user StefanRRichter commented on a diff in the pull request:
https://github.com/apache/flink/pull/5582#discussion_r192070020
--- Diff:
flink-state-backends/flink-statebackend-rocksdb/src/main/java/org/apache/flink/contrib/streaming/state/RocksDBKeySerializationUtils.java
Github user StefanRRichter commented on a diff in the pull request:
https://github.com/apache/flink/pull/5582#discussion_r192052899
--- Diff:
flink-state-backends/flink-statebackend-rocksdb/src/main/java/org/apache/flink/contrib/streaming/state/RocksDBKeySerializationUtils.java
Github user StefanRRichter commented on a diff in the pull request:
https://github.com/apache/flink/pull/5582#discussion_r192018187
--- Diff:
flink-state-backends/flink-statebackend-rocksdb/src/main/java/org/apache/flink/contrib/streaming/state/RocksDBKeySerializationUtils.java
Github user StefanRRichter commented on the issue:
https://github.com/apache/flink/pull/6062
Thanks for the reviews guys! I think I addressed your comments and will
merge now.
---
Github user StefanRRichter commented on the issue:
https://github.com/apache/flink/pull/6077
Thanks for the reviews. Will merge this.
---
Github user StefanRRichter commented on a diff in the pull request:
https://github.com/apache/flink/pull/6062#discussion_r191768328
--- Diff:
flink-streaming-java/src/main/java/org/apache/flink/streaming/api/operators/InternalTimerHeap.java
---
@@ -0,0 +1,511
Github user StefanRRichter commented on a diff in the pull request:
https://github.com/apache/flink/pull/6062#discussion_r191738639
--- Diff:
flink-streaming-java/src/main/java/org/apache/flink/streaming/api/operators/InternalTimerHeap.java
---
@@ -0,0 +1,511
Github user StefanRRichter commented on a diff in the pull request:
https://github.com/apache/flink/pull/6062#discussion_r191732576
--- Diff:
flink-streaming-java/src/test/java/org/apache/flink/streaming/api/operators/InternalTimerHeapTest.java
---
@@ -0,0 +1,470
Github user StefanRRichter commented on a diff in the pull request:
https://github.com/apache/flink/pull/6062#discussion_r191732272
--- Diff:
flink-streaming-java/src/main/java/org/apache/flink/streaming/api/operators/InternalTimersSnapshotReaderWriters.java
---
@@ -96,7 +96,7
Github user StefanRRichter commented on a diff in the pull request:
https://github.com/apache/flink/pull/6062#discussion_r191731222
--- Diff:
flink-streaming-java/src/main/java/org/apache/flink/streaming/api/operators/TimerHeapInternalTimer.java
---
@@ -0,0 +1,246
Github user StefanRRichter commented on a diff in the pull request:
https://github.com/apache/flink/pull/6062#discussion_r191730978
--- Diff:
flink-streaming-java/src/main/java/org/apache/flink/streaming/api/operators/TimerHeapInternalTimer.java
---
@@ -0,0 +1,246
Github user StefanRRichter commented on a diff in the pull request:
https://github.com/apache/flink/pull/6062#discussion_r191711210
--- Diff:
flink-streaming-java/src/main/java/org/apache/flink/streaming/api/operators/HeapInternalTimerService.java
---
@@ -199,17 +186,9 @@ public
Github user StefanRRichter commented on a diff in the pull request:
https://github.com/apache/flink/pull/6062#discussion_r191454466
--- Diff:
flink-streaming-java/src/main/java/org/apache/flink/streaming/api/operators/InternalTimerHeap.java
---
@@ -0,0 +1,511
Github user StefanRRichter commented on a diff in the pull request:
https://github.com/apache/flink/pull/5582#discussion_r191446316
--- Diff:
flink-state-backends/flink-statebackend-rocksdb/src/main/java/org/apache/flink/contrib/streaming/state/RocksDBKeyedStateBackend.java
Github user StefanRRichter commented on a diff in the pull request:
https://github.com/apache/flink/pull/5582#discussion_r19164
--- Diff:
flink-state-backends/flink-statebackend-rocksdb/src/main/java/org/apache/flink/contrib/streaming/state/RocksDBKeySerializationUtils.java
Github user StefanRRichter commented on the issue:
https://github.com/apache/flink/pull/6096
CC @aljoscha or @tillrohrmann
---
GitHub user StefanRRichter opened a pull request:
https://github.com/apache/flink/pull/6096
[FLINK-9440][streaming] Expose cancelation of timers through timer service
interface
## What is the purpose of the change
This PR is enhancing PR #6077 and #6062. The purpose is to
Github user StefanRRichter commented on the issue:
https://github.com/apache/flink/pull/5834
LGTM ð Will rebase and merge once my Travis is green.
---
Github user StefanRRichter commented on the issue:
https://github.com/apache/flink/pull/5820
This looks good to me ð. Will merge.
---
Github user StefanRRichter commented on the issue:
https://github.com/apache/flink/pull/6038
@tzulitai thanks for this additional test. LGTM ð
---
Github user StefanRRichter commented on the issue:
https://github.com/apache/flink/pull/6062
Ok, I also think might still be some room for optimizations of the
implementation for a `RocksDBTimerHeap` over the existing code.
---
Github user StefanRRichter commented on the issue:
https://github.com/apache/flink/pull/6062
@sihuazhou This is exactly my plan on how introduce a "timer state" for the
keyed state backends. In a nutshell, time timer service can just register and
operator on those states,
Github user StefanRRichter commented on the issue:
https://github.com/apache/flink/pull/6077
CC @aljoscha
---
GitHub user StefanRRichter opened a pull request:
https://github.com/apache/flink/pull/6077
FLINK-9436][state] Remove generic parameter namespace from
InternalTimeServiceManager
## Brief change log
This PR removes the misleading generic parameter `N` from
Github user StefanRRichter commented on the issue:
https://github.com/apache/flink/pull/6062
@sihuazhou thanks for the review. Addressed your comments, please take a
look if you want.
---
Github user StefanRRichter commented on a diff in the pull request:
https://github.com/apache/flink/pull/6062#discussion_r190482258
--- Diff:
flink-streaming-java/src/main/java/org/apache/flink/streaming/api/operators/InternalTimerHeap.java
---
@@ -0,0 +1,504
Github user StefanRRichter commented on a diff in the pull request:
https://github.com/apache/flink/pull/6062#discussion_r190481987
--- Diff:
flink-streaming-java/src/main/java/org/apache/flink/streaming/api/operators/TimerHeapInternalTimer.java
---
@@ -0,0 +1,235
Github user StefanRRichter commented on a diff in the pull request:
https://github.com/apache/flink/pull/6062#discussion_r190296235
--- Diff:
flink-streaming-java/src/main/java/org/apache/flink/streaming/api/operators/InternalTimerHeap.java
---
@@ -0,0 +1,504
Github user StefanRRichter commented on a diff in the pull request:
https://github.com/apache/flink/pull/6062#discussion_r190293752
--- Diff:
flink-streaming-java/src/main/java/org/apache/flink/streaming/api/operators/InternalTimerHeap.java
---
@@ -0,0 +1,504
Github user StefanRRichter commented on a diff in the pull request:
https://github.com/apache/flink/pull/6062#discussion_r190289953
--- Diff:
flink-streaming-java/src/main/java/org/apache/flink/streaming/api/operators/InternalTimerHeap.java
---
@@ -0,0 +1,504
Github user StefanRRichter commented on the issue:
https://github.com/apache/flink/pull/5959
LGTM ð
---
Github user StefanRRichter commented on the issue:
https://github.com/apache/flink/pull/5773
LGTM ð
Will merge this.
---
Github user StefanRRichter commented on the issue:
https://github.com/apache/flink/pull/6063
@sihuazhou no problem, I will merge this fixup.
---
Github user StefanRRichter commented on the issue:
https://github.com/apache/flink/pull/6062
CC @aljoscha
---
GitHub user StefanRRichter opened a pull request:
https://github.com/apache/flink/pull/6062
[FLINK-9423][state] Implement efficient deletes for heap-based timer â¦
â¦service.
## What is the purpose of the change
This PR introduces `InternalTimerHeap`, as data
Github user StefanRRichter commented on the issue:
https://github.com/apache/flink/pull/5959
Sounds good. After the mentioned changes this looks ready to merge for me
ð
---
Github user StefanRRichter commented on a diff in the pull request:
https://github.com/apache/flink/pull/5959#discussion_r190168697
--- Diff:
flink-runtime/src/main/java/org/apache/flink/runtime/metrics/groups/ComponentMetricGroup.java
---
@@ -57,11 +57,12 @@ public
Github user StefanRRichter commented on the issue:
https://github.com/apache/flink/pull/5959
I have two quick questions:
- Why does `ComponentMetricGroup` even override the method
`getAllVariables` from `AbstractMetricGroup` with essentially the exact same
code?
-Why is it
Github user StefanRRichter commented on a diff in the pull request:
https://github.com/apache/flink/pull/5959#discussion_r190165634
--- Diff:
flink-runtime/src/main/java/org/apache/flink/runtime/metrics/groups/ComponentMetricGroup.java
---
@@ -57,11 +57,12 @@ public
Github user StefanRRichter commented on a diff in the pull request:
https://github.com/apache/flink/pull/5959#discussion_r190165058
--- Diff:
flink-runtime/src/main/java/org/apache/flink/runtime/metrics/groups/ComponentMetricGroup.java
---
@@ -57,11 +57,12 @@ public
Github user StefanRRichter commented on the issue:
https://github.com/apache/flink/pull/5650
@sihuazhou thanks for this nice contribution. LGTM ð Will merge.
---
Github user StefanRRichter commented on the issue:
https://github.com/apache/flink/pull/3359
@shixiaogang I had a brief look into the updated PR. It seems like this
current version does not include integrating timers with the other keyed state?
I am asking because my plan would be to
Github user StefanRRichter commented on the issue:
https://github.com/apache/flink/pull/5908
@makeyang can you also please close this PR?
---
Github user StefanRRichter commented on the issue:
https://github.com/apache/flink/pull/6025
@zhgao this looks like a mistake. Can you please close the PR?
---
Github user StefanRRichter commented on the issue:
https://github.com/apache/flink/pull/5979
LGTM, will merge.
---
Github user StefanRRichter commented on the issue:
https://github.com/apache/flink/pull/5979
With the approach I outlined, we would not require any `seek()` to the last
key, we can simply create the exclusive end key. Nevertheless, you are right
about the comment that is only in the
Github user StefanRRichter commented on the issue:
https://github.com/apache/flink/pull/5979
@sihuazhou I wonder why you would chose iterator + batched write over
simply calling `db.deleteRange(...)` where start key is
`serializeCurrentKeyAndNamespace()` and end key is increasing the
Github user StefanRRichter commented on the issue:
https://github.com/apache/flink/pull/6020
Thanks @sihuazhou ! LGTM ð Will merge this.
---
Github user StefanRRichter commented on the issue:
https://github.com/apache/flink/pull/5676
Thanks for the feedback, I have addressed the last comments and will merge
this.
---
Github user StefanRRichter commented on the issue:
https://github.com/apache/flink/pull/6020
Let's wait a bit more for their response. It seems like this example is
older than their corrected docs.
---
Github user StefanRRichter commented on the issue:
https://github.com/apache/flink/pull/6020
Maybe we should ask them on their issue tracker what the best practise is?
I cannot remember seeing such checks in their code examples. Have a hard time
to believe that this can be true
Github user StefanRRichter commented on the issue:
https://github.com/apache/flink/pull/6020
It depends, maybe this is already covered currently because we might always
do an iteration attempt that checks right after the seek. But in general, this
is not very nice and fragile if true.
---
Github user StefanRRichter commented on the issue:
https://github.com/apache/flink/pull/6020
After double checking with the RocksDB docs, I am afraid that we need to
introduce more checks because for example the point out that also after methods
like `seek` the iterator an become
Github user StefanRRichter commented on the issue:
https://github.com/apache/flink/pull/6020
It sounds cheap if they just `&` all the flags from the sub iterators. In
the end, we can see if there is a performance drop but better be correct first
before fast.
---
Github user StefanRRichter commented on the issue:
https://github.com/apache/flink/pull/6020
Oh you are right, this is confusing :-) So does this also mean the status
flag is cleared when we simple continue iterating and only check in the end?
---
Github user StefanRRichter commented on the issue:
https://github.com/apache/flink/pull/6020
Also from the RocksDB docs: `In another word, if Iterator::Valid() is true,
status() is guaranteed to be OK()`
---
Github user StefanRRichter commented on the issue:
https://github.com/apache/flink/pull/6020
Yes, but eventually it will also return `false`, which is essentially the
same as waiting until the loop terminates. Anyways, I think after the loop is
the nicer way.
---
Github user StefanRRichter commented on the issue:
https://github.com/apache/flink/pull/6020
You could also in `isRocksIteratorValid` run the check only if the return
value is `false` if you like the helper method to avoid people forgetting about
this check.
---
Github user StefanRRichter commented on the issue:
https://github.com/apache/flink/pull/6020
A quick general question: could you observe any performance impact from
calling the `status()` method in the loops. It looks like a native method and I
am not sure that it is inexpensive
Github user StefanRRichter commented on the issue:
https://github.com/apache/flink/pull/6019
Yes, the only reason I did not start with this is that I first wanted to
wait for the completion and merge of the RocksDB timer service to have a
complete picture.
---
Github user StefanRRichter commented on the issue:
https://github.com/apache/flink/pull/6019
@sihuazhou my plan was to integrate the timer service more closely with the
keyed state backends, starting from the point that we are merging the PR for
timers in RocksDB. I think timers
Github user StefanRRichter commented on the issue:
https://github.com/apache/flink/pull/5977
Overall LGTM ð
---
Github user StefanRRichter commented on a diff in the pull request:
https://github.com/apache/flink/pull/5977#discussion_r188221002
--- Diff:
flink-core/src/main/java/org/apache/flink/api/common/functions/RuntimeContext.java
---
@@ -71,6 +71,17 @@
@PublicEvolving
Github user StefanRRichter commented on a diff in the pull request:
https://github.com/apache/flink/pull/5977#discussion_r188220497
--- Diff:
flink-core/src/main/java/org/apache/flink/api/common/functions/RuntimeContext.java
---
@@ -71,6 +71,17 @@
@PublicEvolving
Github user StefanRRichter commented on the issue:
https://github.com/apache/flink/pull/3359
@shixiaogang thanks a lot! I will try to take a look at this as soon as
possible.
---
Github user StefanRRichter commented on a diff in the pull request:
https://github.com/apache/flink/pull/5962#discussion_r188010236
--- Diff:
flink-streaming-java/src/main/java/org/apache/flink/streaming/runtime/tasks/SystemProcessingTimeService.java
---
@@ -197,6 +204,23
Github user StefanRRichter commented on a diff in the pull request:
https://github.com/apache/flink/pull/5962#discussion_r188006850
--- Diff:
flink-streaming-java/src/test/java/org/apache/flink/streaming/runtime/tasks/SystemProcessingTimeServiceTest.java
---
@@ -504,7 +475,101
Github user StefanRRichter commented on the issue:
https://github.com/apache/flink/pull/6006
Thanks for the reviews. Will merge this.
---
Github user StefanRRichter commented on a diff in the pull request:
https://github.com/apache/flink/pull/6006#discussion_r188002995
--- Diff:
flink-runtime/src/main/java/org/apache/flink/runtime/taskexecutor/TaskManagerServicesConfiguration.java
---
@@ -128,8 +128,8 @@ public
Github user StefanRRichter commented on a diff in the pull request:
https://github.com/apache/flink/pull/6006#discussion_r188002868
--- Diff:
flink-runtime/src/main/java/org/apache/flink/runtime/taskexecutor/TaskManagerServicesConfiguration.java
---
@@ -78,13 +78,13
Github user StefanRRichter commented on the issue:
https://github.com/apache/flink/pull/6006
@zentol thanks for pointing that out. Updated the PR.
---
Github user StefanRRichter commented on the issue:
https://github.com/apache/flink/pull/5962
Thanks for the comments, I have updated the PR with another commit. Please
take a look again if this can be merged.
---
GitHub user StefanRRichter opened a pull request:
https://github.com/apache/flink/pull/6006
[FLINK-9355][checkpointing] Simplify configuration of local recovery â¦
â¦to a simple on/off switch
## What is the purpose of the change
This PR makes the configuration of
Github user StefanRRichter commented on the issue:
https://github.com/apache/flink/pull/6006
CC @tillrohrmann
---
Github user StefanRRichter commented on the issue:
https://github.com/apache/flink/pull/5749
What exactly is the benefit of this change? Or why `Iterable` and not
`Collection` so that we can benefit from methods like `addAll(...)` which can
optimize insertion because the size is
Github user StefanRRichter commented on the issue:
https://github.com/apache/flink/pull/5941
As discussed separately, I think that this could be simplified quiet a bit
and that we do not require connected streams. The operator state does not have
to depend on the input events. For
Github user StefanRRichter commented on a diff in the pull request:
https://github.com/apache/flink/pull/5941#discussion_r186415017
--- Diff:
flink-end-to-end-tests/flink-datastream-allround-test/src/main/java/org/apache/flink/streaming/tests/SequenceGeneratorSource.java
Github user StefanRRichter commented on the issue:
https://github.com/apache/flink/pull/5926
LGTM ð
---
Github user StefanRRichter commented on a diff in the pull request:
https://github.com/apache/flink/pull/5926#discussion_r186412353
--- Diff: flink-end-to-end-tests/run-nightly-tests.sh ---
@@ -58,25 +58,97 @@ fi
if [ $EXIT_CODE == 0 ]; then
printf
&qu
Github user StefanRRichter commented on the issue:
https://github.com/apache/flink/pull/5962
CC @tillrohrmann
---
GitHub user StefanRRichter opened a pull request:
https://github.com/apache/flink/pull/5962
[FLINK-9304] Timer service shutdown should not stop if interrupted
## What is the purpose of the change
This PR prevents that interruption prematurely stops the shutdown of the
Github user StefanRRichter commented on the issue:
https://github.com/apache/flink/pull/5947
LGTM ð Will merge this.
---
Github user StefanRRichter commented on the issue:
https://github.com/apache/flink/pull/5921
In that case, LGTM ð Will merge this.
---
Github user StefanRRichter commented on the issue:
https://github.com/apache/flink/pull/5950
Overall, I think this looks good for me now ð
---
Github user StefanRRichter commented on a diff in the pull request:
https://github.com/apache/flink/pull/5950#discussion_r186082804
--- Diff:
flink-core/src/main/java/org/apache/flink/api/common/typeutils/TypeSerializerSerializationUtil.java
---
@@ -102,17 +99,24
Github user StefanRRichter commented on the issue:
https://github.com/apache/flink/pull/5950
Yes, I think we can only remove the flag further when splitting up
`readSerializersAndConfigsWithResilience`, but I guess it is ok to leave it if
we change this soon anyways.
---
Github user StefanRRichter commented on the issue:
https://github.com/apache/flink/pull/5947
Please also double check, it seems there are files without license header.
---
Github user StefanRRichter commented on a diff in the pull request:
https://github.com/apache/flink/pull/5947#discussion_r185840683
--- Diff:
flink-end-to-end-tests/flink-datastream-allround-test/src/main/java/org/apache/flink/streaming/tests/artificialstate/eventpayload
Github user StefanRRichter commented on a diff in the pull request:
https://github.com/apache/flink/pull/5947#discussion_r185840213
--- Diff:
flink-end-to-end-tests/flink-datastream-allround-test/src/main/java/org/apache/flink/streaming/tests/artificialstate/eventpayload
Github user StefanRRichter commented on the issue:
https://github.com/apache/flink/pull/5950
Please also check the travis build, some related tests seem to fail:
Tests in error
Github user StefanRRichter commented on a diff in the pull request:
https://github.com/apache/flink/pull/5950#discussion_r185828137
--- Diff:
flink-core/src/main/java/org/apache/flink/api/common/typeutils/TypeSerializerSerializationUtil.java
---
@@ -200,7 +197,7 @@ public static
Github user StefanRRichter commented on a diff in the pull request:
https://github.com/apache/flink/pull/5950#discussion_r185825767
--- Diff:
flink-core/src/main/java/org/apache/flink/api/common/typeutils/TypeSerializerSerializationUtil.java
---
@@ -200,7 +197,7 @@ public static
Github user StefanRRichter commented on a diff in the pull request:
https://github.com/apache/flink/pull/5950#discussion_r185821296
--- Diff:
flink-core/src/main/java/org/apache/flink/api/common/typeutils/TypeSerializerSerializationUtil.java
---
@@ -373,15 +370,14 @@ public void
201 - 300 of 1461 matches
Mail list logo
