Github user sihuazhou commented on the issue:
https://github.com/apache/flink/pull/5650
A micro-benchmark for this: @StefanRRichter @StephanEwen
```
---> With disableWAL is false <-
Number of values added | time f
Github user sihuazhou commented on the issue:
https://github.com/apache/flink/pull/6038
Oh this is great! @StefanRRichter Is this means that flink plan to make
checkpoint support rescaling officially in the near future?
---
Github user sihuazhou commented on the issue:
https://github.com/apache/flink/pull/5979
Thank you all @StefanRRichter @StephanEwen @bowenli86
---
Github user sihuazhou commented on the issue:
https://github.com/apache/flink/pull/5979
Hi @StefanRRichter I rebased the PR, could you please have a look?
---
Github user sihuazhou commented on the issue:
https://github.com/apache/flink/pull/5979
Hi @StefanRRichter If I'm not sure whether we can do that without `seek()`,
because the `key bytes` is length is not fixed which may lead to delete
wrongly, What do you think?
Github user sihuazhou commented on the issue:
https://github.com/apache/flink/pull/5979
Hmm...there is another reason, indeed the mainly performance overhead is
the `seek()`. Even though we use the `deleteRange()` to implement this, we also
need to get the last key of the entries
Github user sihuazhou commented on the issue:
https://github.com/apache/flink/pull/5979
@StefanRRichter , the reason I prefer this approach is that:
- From the comment in RocksDB's source we can find that deleteRange()
should be used for deleting big range, what i
Github user sihuazhou commented on the issue:
https://github.com/apache/flink/pull/5847
Closing, because it's already merged.
---
Github user sihuazhou closed the pull request at:
https://github.com/apache/flink/pull/5847
---
Github user sihuazhou commented on the issue:
https://github.com/apache/flink/pull/6020
cc @StefanRRichter
---
Github user sihuazhou commented on the issue:
https://github.com/apache/flink/pull/6020
Have't received any response from RocksDB yet, but I found this example
with using `RocksIterator#stats()`:
https://github.com/facebook/rocksdb/blob/3453870677ee2648f38d70fe8aa7fa16a93a96d2
Github user sihuazhou commented on the issue:
https://github.com/apache/flink/pull/5979
@StephanEwen , I had a micro-benchmark, here is the result
```
-> Batch VS Put <
BATCH: end insert - duration:255
PUT: end insert - durati
Github user sihuazhou commented on the issue:
https://github.com/apache/flink/pull/6020
FYI, I found this issue related to problem:
https://github.com/facebook/rocksdb/issues/3558
---
Github user sihuazhou commented on the issue:
https://github.com/apache/flink/pull/6020
Agreed!
---
Github user sihuazhou commented on the issue:
https://github.com/apache/flink/pull/6020
Oh My God...Is that means we need to wrap the `RocksIterator` to delegate
all it API?
---
Github user sihuazhou commented on the issue:
https://github.com/apache/flink/pull/6020
Agree, should be correct first before fast! Could you please have a look at
this? I think it's already for a look now~
---
Github user sihuazhou commented on the issue:
https://github.com/apache/flink/pull/6020
@StefanRRichter I had a look at the implementation of the iterators in
RocksDB, I found status just return the flag first `_status` as the result
without any complex computation, But for some
Github user sihuazhou commented on the issue:
https://github.com/apache/flink/pull/6020
I'm going to check the native implementation and see whether the `status()`
is a super cheap option...
---
Github user sihuazhou commented on the issue:
https://github.com/apache/flink/pull/6020
I think I am a bit torn here now...
---
Github user sihuazhou commented on the issue:
https://github.com/apache/flink/pull/6020
That is a good question, and I'm not sure...but I think that seems to be...
---
Github user sihuazhou commented on the issue:
https://github.com/apache/flink/pull/6020
I think that is the incorrect one, If I'm not confused by the wiki's
content...
---
Github user sihuazhou commented on the issue:
https://github.com/apache/flink/pull/6020
@StefanRRichter NO, I think that couldn't fix this issue, the problem here
is that even `iterator.isValid()` return `true`, there may also some internal
error in RocksDB. What do you think?
---
Github user sihuazhou commented on the issue:
https://github.com/apache/flink/pull/6020
@StefanRRichter No, I didn't have any performance tests yet. I think you
are right! Your proposal is the way I'm going to choose. Addressing this...
---
Github user sihuazhou commented on the issue:
https://github.com/apache/flink/pull/6019
@StefanRRichter I definitely agree with your point! The timers should be
considered as keyed state is a beautiful way to go!
---
Github user sihuazhou commented on the issue:
https://github.com/apache/flink/pull/6020
The reasons that the travis given red light is unrelated...
---
Github user sihuazhou commented on the issue:
https://github.com/apache/flink/pull/6020
cc @StefanRRichter
---
Github user sihuazhou commented on the issue:
https://github.com/apache/flink/pull/6019
I wonder can you introduce a `HeapState` which scoped to `key group` to
support timer service. This way timer service is backed by keyed state backend,
which looks like a beautiful things.
---
GitHub user sihuazhou opened a pull request:
https://github.com/apache/flink/pull/6020
[FLINK-9373][state] Always call RocksIterator.status() to check the
internal error of RocksDB
## What is the purpose of the change
Currently, when using RocksIterator we only use the
Github user sihuazhou commented on the issue:
https://github.com/apache/flink/pull/5982
@StephanEwen I guess this PR is already for an another look now...
---
Github user sihuazhou commented on the issue:
https://github.com/apache/flink/pull/5879
Could anyone have a look at this? A minor change that would only effects
the log...absolutely won't cause any regression problem, but would be helpful
when user diagnosing the job according t
Github user sihuazhou commented on the issue:
https://github.com/apache/flink/pull/5982
@StephanEwen Thanks for your good suggestions! Will update PR like it, and
what about the problem related to WriteMode.OVERWRITE, do you against if we
don't support it in `createAtomically()`?
---
Github user sihuazhou commented on the issue:
https://github.com/apache/flink/pull/5847
cc @StefanRRichter could you please have a look at this...
---
Github user sihuazhou commented on the issue:
https://github.com/apache/flink/pull/5847
cc @tillrohrmann could you please take a look at this?
---
Github user sihuazhou commented on the issue:
https://github.com/apache/flink/pull/5982
Hi, I met a problem here, for the Hadoop FileSystem when the
`schema='hdfs'`, we can't rename a file to a existing file. This means that it
hard(I'm not sure whether it's
Github user sihuazhou commented on the issue:
https://github.com/apache/flink/pull/5982
Hi @StephanEwen, I added more tests for the
`FileSystem#createAtomically()`, concerning the `TwoPhaseFsDataoutputStream`,
can we introduce a `commit_on_close` option for it to make it easier to
Github user sihuazhou commented on the issue:
https://github.com/apache/flink/pull/5879
@tillrohrmann I updated the PR, could you please have a look again?
---
Github user sihuazhou commented on the issue:
https://github.com/apache/flink/pull/5879
@tillrohrmann I think would be enough to simply print the exception message
in the log, if we will refactor the PRC as a second step. Will update the PR.
---
Github user sihuazhou commented on the issue:
https://github.com/apache/flink/pull/5982
@StephanEwen After thinking your comments again, I think I misunderstood
the `Also, we need this method to be implemented in all FileSystem subclasses.`
ð , I will address that. But the
Github user sihuazhou commented on the issue:
https://github.com/apache/flink/pull/5982
Hi @StephanEwen Thank you very much for your reply, I'm not sure whether
just overriding `close()` to do `super.close()` + `rename()` is enough, for
example.
```
try (outputStream
Github user sihuazhou commented on the issue:
https://github.com/apache/flink/pull/5879
Hi @tillrohrmann , thanks for your reply, @zentol proposed to introduce a
`normal-life-cycle exception` in his previous review, the `normal-left-cycle
exception` would only log the exception
Github user sihuazhou commented on the issue:
https://github.com/apache/flink/pull/6001
hi @medcv I think maybe it better to assign the issue to yourself before
working on that, this could help to avoid depulicate works(cause i notice that
you make the PRs without assigning to
Github user sihuazhou commented on the issue:
https://github.com/apache/flink/pull/5847
Could anyone have a look at this...
---
Github user sihuazhou commented on the issue:
https://github.com/apache/flink/pull/5879
cc @StephanEwen
---
Github user sihuazhou commented on the issue:
https://github.com/apache/flink/pull/5982
Hi @StephanEwen I have update the PR according to the above comments, it's
ready for an another review.
---
Github user sihuazhou commented on the issue:
https://github.com/apache/flink/pull/5993
I made an additional fixup to `StateBackendTestBase#testMapState()` to
guard this fix in the future, @kl0u please free feel to let me know if you want
to revert it. I do this because this bug
Github user sihuazhou commented on the issue:
https://github.com/apache/flink/pull/5982
Hi @StephanEwen,
- I prefer to introduce the `create(Path, WriteMode)`, because I feel this
is more extensible (it could allow us to introduce other WriteMode's in the
future).
- I
Github user sihuazhou commented on the issue:
https://github.com/apache/flink/pull/5993
cc @StefanRRichter, @kl0u you may also care about this...
---
GitHub user sihuazhou opened a pull request:
https://github.com/apache/flink/pull/5993
[FLINK-9336][state] fix deserialization problem for queryable MapState
## What is the purpose of the change
This PR fixes the deserialization problem for queryable MapState. Currently
Github user sihuazhou commented on the issue:
https://github.com/apache/flink/pull/5982
@StephanEwen Thank you very much for your great suggestion! I will address
this that way.
---
Github user sihuazhou commented on the issue:
https://github.com/apache/flink/pull/5928
Hi guys, I think maybe the doc that related to the `Externalized
Checkpoint` should be updated for 1.5. Are there any other suggestions? And if
there is anyone who is willing to take over this plz
Github user sihuazhou commented on a diff in the pull request:
https://github.com/apache/flink/pull/5931#discussion_r187514050
--- Diff:
flink-yarn/src/main/java/org/apache/flink/yarn/YarnResourceManager.java ---
@@ -293,21 +293,16 @@ public void startNewWorker(ResourceProfile
GitHub user sihuazhou opened a pull request:
https://github.com/apache/flink/pull/5987
[FLINK-9043][CLI]Automatically search for the last successful checkpoint
when recover the job from externalized checkpoint
## What is the purpose of the change
*Automatically search for
Github user sihuazhou commented on the issue:
https://github.com/apache/flink/pull/5982
Hi @StephanEwen Could you please have a look at this?
---
Github user sihuazhou commented on a diff in the pull request:
https://github.com/apache/flink/pull/5979#discussion_r187278034
--- Diff:
flink-state-backends/flink-statebackend-rocksdb/src/main/java/org/apache/flink/contrib/streaming/state/RocksDBMapState.java
---
@@ -356,6
GitHub user sihuazhou opened a pull request:
https://github.com/apache/flink/pull/5982
[FLINK-9325][checkpoint]generate the meta file for checkpoint only when the
writing is truly successful
## What is the purpose of the change
*This PR in order to enhance the checkpoint
Github user sihuazhou commented on a diff in the pull request:
https://github.com/apache/flink/pull/5931#discussion_r187227128
--- Diff:
flink-yarn/src/main/java/org/apache/flink/yarn/YarnResourceManager.java ---
@@ -293,21 +293,16 @@ public void startNewWorker(ResourceProfile
Github user sihuazhou commented on the issue:
https://github.com/apache/flink/pull/5979
cc @StefanRRichter (This is for 1.6, I just complete it when I have time
currently)
---
Github user sihuazhou commented on the issue:
https://github.com/apache/flink/pull/5879
cc @tillrohrmann
---
GitHub user sihuazhou opened a pull request:
https://github.com/apache/flink/pull/5979
[FLINK-9070][state]improve the performance of RocksDBMapState.clear()
## What is the purpose of the change
This PR intend to improve the performance of `RocksDBMapState.clear()` base
on
Github user sihuazhou commented on the issue:
https://github.com/apache/flink/pull/5847
cc @StephanEwen Could you please have a look at this? I think it's already
for another review.
---
Github user sihuazhou commented on a diff in the pull request:
https://github.com/apache/flink/pull/5962#discussion_r186775375
--- Diff:
flink-streaming-java/src/main/java/org/apache/flink/streaming/runtime/tasks/StreamTask.java
---
@@ -706,6 +689,38 @@ public void
Github user sihuazhou commented on a diff in the pull request:
https://github.com/apache/flink/pull/5966#discussion_r186769177
--- Diff:
flink-runtime/src/main/java/org/apache/flink/runtime/io/network/netty/NettyServer.java
---
@@ -170,8 +171,8 @@ public void initChannel
Github user sihuazhou commented on the issue:
https://github.com/apache/flink/pull/5749
Hi, @StefanRRichter That's a good question, I think the `Collection` looks
better then `Iterable`... the benefit is that the api looks more consistency
with the java api, and maybe convenie
Github user sihuazhou commented on the issue:
https://github.com/apache/flink/pull/5749
Hey guys, what going on with this now? I think this is a pretty good
ideal...
---
Github user sihuazhou commented on the issue:
https://github.com/apache/flink/pull/5847
Hi, @aljoscha will this PR go into 1.5?
---
Github user sihuazhou commented on the issue:
https://github.com/apache/flink/pull/5931
I also `+1` for the first approach, but I would like to wait for
@tillrohrmann 's opinion.
And I also curious about one thing, that is currently when ResourceManager
allocate a slo
Github user sihuazhou commented on the issue:
https://github.com/apache/flink/pull/5879
Hi @zentol , will this PR go into 1.5? I think the log seems to be a bit
noise currently when we diagnosing the job.
---
Github user sihuazhou commented on the issue:
https://github.com/apache/flink/pull/5931
Hi @shuai-xu, If I'm not misunderstand, I think your approach is exactly
what I have done in the previous
[PR](https://github.com/apache/flink/pull/5881) for this ticket, but it faces
the
Github user sihuazhou commented on the issue:
https://github.com/apache/flink/pull/5847
Hi @aljoscha does this PR look good to you now?
---
Github user sihuazhou commented on the issue:
https://github.com/apache/flink/pull/5928
@zentol I have addressed your comments.
---
Github user sihuazhou commented on a diff in the pull request:
https://github.com/apache/flink/pull/5934#discussion_r185486283
--- Diff:
flink-runtime/src/main/java/org/apache/flink/runtime/state/heap/HeapKeyedStateBackend.java
---
@@ -622,20 +622,23 @@ public
Github user sihuazhou commented on a diff in the pull request:
https://github.com/apache/flink/pull/5934#discussion_r185486196
--- Diff:
flink-state-backends/flink-statebackend-rocksdb/src/main/java/org/apache/flink/contrib/streaming/state/RocksDBKeyedStateBackend.java
Github user sihuazhou commented on a diff in the pull request:
https://github.com/apache/flink/pull/5934#discussion_r185485532
--- Diff:
flink-state-backends/flink-statebackend-rocksdb/src/main/java/org/apache/flink/contrib/streaming/state/RocksDBKeyedStateBackend.java
Github user sihuazhou commented on a diff in the pull request:
https://github.com/apache/flink/pull/5934#discussion_r185485401
--- Diff:
flink-runtime/src/test/java/org/apache/flink/runtime/state/StateBackendTestBase.java
---
@@ -3594,6 +3599,58 @@ public String fold(String acc
Github user sihuazhou commented on the issue:
https://github.com/apache/flink/pull/5934
Hi @StefanRRichter Thanks for your reply, I have updated the PR and add a
test to guard this, but I not sure whether the test is indeed required because
it looks a bit wired in my mind...
---
Github user sihuazhou commented on the issue:
https://github.com/apache/flink/pull/5928
@StephanEwen @alpinegizmo Thanks for your comments, I will revert the
changes of `config.md` and address the comments concerning to the ``` `file://`
only for local setups```.
---
Github user sihuazhou commented on the issue:
https://github.com/apache/flink/pull/5934
Thanks for your comments, @StephanEwen , If I am not misunderstanding , we
don't need to duplicate the serializer now, because we will have a dedicated
optimization for it in the near future,
Github user sihuazhou commented on the issue:
https://github.com/apache/flink/pull/5931
Hi @GJL , is it possible that the reason is the same as in the previous PR
for this ticket, that is even the container setup successfully and connect with
ResourceManager successfully, but the TM
Github user sihuazhou commented on the issue:
https://github.com/apache/flink/pull/5847
@aljoscha Thanks for your review, I have addressed your comments.
---
Github user sihuazhou commented on the issue:
https://github.com/apache/flink/pull/5934
Hi @StefanRRichter , I think I feel lost again... when I writing the
comments for the serializer about why we don't duplicate it, I found a loophole
there. In theory, even tough the serializ
Github user sihuazhou commented on the issue:
https://github.com/apache/flink/pull/5934
About the serializer duplication problem, I think you are right,
duplicating a serialize is not always super cheap, so I think maybe the best
tradeoff is to not duplicate the serializer to save
Github user sihuazhou commented on the issue:
https://github.com/apache/flink/pull/5934
Hi @StefanRRichter sorry for the unclearly description here. What this PR
trying to fix is the mainly relate to the below code which run async:
```java
for (Map.Entry> kvSt
GitHub user sihuazhou opened a pull request:
https://github.com/apache/flink/pull/5934
[FLINK-9269][state] fix concurrency problem when performing checkpoint in
HeapKeyedStateBackend
## What is the purpose of the change
*This PR fixes the concurrency problem in
Github user sihuazhou commented on a diff in the pull request:
https://github.com/apache/flink/pull/5931#discussion_r184832034
--- Diff:
flink-yarn/src/main/java/org/apache/flink/yarn/YarnResourceManager.java ---
@@ -293,21 +293,16 @@ public void startNewWorker(ResourceProfile
Github user sihuazhou commented on a diff in the pull request:
https://github.com/apache/flink/pull/5931#discussion_r184831311
--- Diff:
flink-yarn/src/main/java/org/apache/flink/yarn/YarnResourceManager.java ---
@@ -293,21 +293,16 @@ public void startNewWorker(ResourceProfile
Github user sihuazhou commented on the issue:
https://github.com/apache/flink/pull/5930
@StefanRRichter Sure, that sounds good, thanks for your supplement.
---
Github user sihuazhou commented on the issue:
https://github.com/apache/flink/pull/5930
@StefanRRichter Thanks for your nice reviews, addressed your comments.
---
Github user sihuazhou commented on a diff in the pull request:
https://github.com/apache/flink/pull/5930#discussion_r184709505
--- Diff:
flink-runtime/src/main/java/org/apache/flink/runtime/state/RegisteredBroadcastBackendStateMetaInfo.java
---
@@ -52,6 +52,23 @@ public
Github user sihuazhou commented on a diff in the pull request:
https://github.com/apache/flink/pull/5930#discussion_r184709515
--- Diff:
flink-runtime/src/main/java/org/apache/flink/runtime/state/RegisteredBroadcastBackendStateMetaInfo.java
---
@@ -52,6 +52,23 @@ public
Github user sihuazhou closed the pull request at:
https://github.com/apache/flink/pull/5881
---
Github user sihuazhou commented on the issue:
https://github.com/apache/flink/pull/5881
Hi @GJL I see you took over this ticket, I closing this PR now, looking
forward your PR ;).
---
Github user sihuazhou commented on the issue:
https://github.com/apache/flink/pull/5928
Travis failed is unrelated, it cause by the concurrency problem in
`DefaultOperatorStateBackend` when performing checkpoint, which this
[PR](https://github.com/apache/flink/pull/5930) try to fix.
---
Github user sihuazhou commented on the issue:
https://github.com/apache/flink/pull/5930
Hi @StefanRRichter, Could you please have a look at this?
---
GitHub user sihuazhou opened a pull request:
https://github.com/apache/flink/pull/5930
[FLINK-9263][state] Fix concurrency problem in DefaultOperatorStateBackend.
## What is the purpose of the change
This PR fixes the concurrency problem in `DefaultOperatorStateBackend
Github user sihuazhou commented on the issue:
https://github.com/apache/flink/pull/5928
cc @StefanRRichter @StephanEwen
---
GitHub user sihuazhou opened a pull request:
https://github.com/apache/flink/pull/5928
[hotfix][doc] fix doc of externalized checkpoint
## What is the purpose of the change
This PR intend to fix the incorrect doc of externalized checkpoint.
## Brief change log
Github user sihuazhou closed the pull request at:
https://github.com/apache/flink/pull/5911
---
Github user sihuazhou commented on the issue:
https://github.com/apache/flink/pull/5911
hmm...so, closing ...
---
GitHub user sihuazhou opened a pull request:
https://github.com/apache/flink/pull/5911
[FLINK-9243][tests]harden
SuccessAfterNetworkBuffersFailureITCase#testSuccessfulProgramAfterFailure()
## What is the purpose of the change
This PR aim to harden
Github user sihuazhou commented on the issue:
https://github.com/apache/flink/pull/5911
cc @zentol
---
201 - 300 of 536 matches
Mail list logo