Github user sihuazhou commented on a diff in the pull request:
https://github.com/apache/flink/pull/5074#discussion_r153492995
--- Diff:
flink-runtime/src/main/java/org/apache/flink/runtime/checkpoint/savepoint/SavepointV2Serializer.java
---
@@ -510,6 +512,13 @@ private static
Github user sihuazhou commented on a diff in the pull request:
https://github.com/apache/flink/pull/5074#discussion_r153493396
--- Diff:
flink-runtime/src/main/java/org/apache/flink/runtime/taskexecutor/TaskManagerServices.java
---
@@ -190,6 +199,11 @@ public static
Github user sihuazhou commented on a diff in the pull request:
https://github.com/apache/flink/pull/5074#discussion_r153492082
--- Diff:
flink-runtime/src/main/java/org/apache/flink/runtime/checkpoint/savepoint/SavepointV2Serializer.java
---
@@ -510,6 +512,13 @@ private static
Github user sihuazhou commented on a diff in the pull request:
https://github.com/apache/flink/pull/5074#discussion_r153493925
--- Diff:
flink-runtime/src/main/java/org/apache/flink/runtime/state/CachedCheckpointStreamFactory.java
---
@@ -0,0 +1,195 @@
+/*
+ * Licensed
Github user sihuazhou commented on a diff in the pull request:
https://github.com/apache/flink/pull/5074#discussion_r153495140
--- Diff:
flink-runtime/src/main/java/org/apache/flink/runtime/state/CachedCheckpointStreamFactory.java
---
@@ -0,0 +1,195 @@
+/*
+ * Licensed
GitHub user sihuazhou opened a pull request:
https://github.com/apache/flink/pull/5074
[FLINK-7873] [runtime] Introduce local recovery
## What is the purpose of the change
This PR fixes
[FLINK-7873](https://issues.apache.org/jira/browse/FLINK-7873). Current recover
Github user sihuazhou commented on the issue:
https://github.com/apache/flink/pull/5074
@StefanRRichter Could you please have a look at this?
---
GitHub user sihuazhou opened a pull request:
https://github.com/apache/flink/pull/4949
[FLINK-7866] [runtime] Weigh list of preferred locations for scheduling
## What is the purpose of the change
This PR fixs
[FLINK-7866](https://issues.apache.org/jira/browse/FLINK-7866
Github user sihuazhou commented on a diff in the pull request:
https://github.com/apache/flink/pull/4916#discussion_r148211942
--- Diff:
flink-runtime/src/main/java/org/apache/flink/runtime/executiongraph/ExecutionGraph.java
---
@@ -878,113 +880,70 @@ private void scheduleEager
Github user sihuazhou commented on a diff in the pull request:
https://github.com/apache/flink/pull/4916#discussion_r148434074
--- Diff:
flink-runtime/src/main/java/org/apache/flink/runtime/executiongraph/ExecutionGraph.java
---
@@ -878,113 +880,70 @@ private void scheduleEager
Github user sihuazhou commented on a diff in the pull request:
https://github.com/apache/flink/pull/4916#discussion_r148437379
--- Diff:
flink-runtime/src/main/java/org/apache/flink/runtime/executiongraph/ExecutionVertex.java
---
@@ -476,14 +482,13 @@ else if (numSources
Github user sihuazhou commented on the issue:
https://github.com/apache/flink/pull/4949
Hi, @tillrohrmann Could you please have a rough look at this ? I'm not sure
this is you wanted.
---
GitHub user sihuazhou opened a pull request:
https://github.com/apache/flink/pull/4979
RMQSource support disabling queue declaration
## What is the purpose of the change
This PR fixs
[FLINK-8018](https://issues.apache.org/jira/browse/FLINK-8018), RabbitMQ
connector should
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/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
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 = new
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
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
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 possible) to support
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 slot
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/5847
cc @StephanEwen Could you please have a look at this? I think it's already
for another review.
---
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/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 commented on the issue:
https://github.com/apache/flink/pull/5928
cc @StefanRRichter @StephanEwen
---
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 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 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 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/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_r184709515
--- 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_r184709505
--- Diff:
flink-runtime/src/main/java/org/apache/flink/runtime/state/RegisteredBroadcastBackendStateMetaInfo.java
---
@@ -52,6 +52,23 @@ public
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 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 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
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<String, StateTabl
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 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 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's
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
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
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/5879
cc @StephanEwen
---
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/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/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
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 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/5993
cc @StefanRRichter, @kl0u you may also care about this...
---
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 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 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 would
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 convenient
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 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 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 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/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/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 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/6020
cc @StefanRRichter
---
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
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/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/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/6020
cc @StefanRRichter
---
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?
Sure
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
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/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
Agreed!
---
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
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
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/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
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
The reasons that the travis given red light is unrelated...
---
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/6020
I think I am a bit torn here now...
---
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
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
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/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/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
Github user sihuazhou commented on the issue:
https://github.com/apache/flink/pull/5582
Unfortunately, after confirming with RocksDB, the `deleteRange()` is still
an experimental feature, it may have impact on read performance currently(event
thought we could use the ReadOption
Github user sihuazhou commented on a diff in the pull request:
https://github.com/apache/flink/pull/6062#discussion_r190463723
--- Diff:
flink-streaming-java/src/main/java/org/apache/flink/streaming/api/operators/TimerHeapInternalTimer.java
---
@@ -0,0 +1,235
Github user sihuazhou commented on the issue:
https://github.com/apache/flink/pull/6063
cc @StefanRRichter I'm sorry for introduce this unstable tests...
---
Github user sihuazhou commented on a diff in the pull request:
https://github.com/apache/flink/pull/6062#discussion_r190517094
--- Diff:
flink-streaming-java/src/main/java/org/apache/flink/streaming/api/operators/InternalTimerHeap.java
---
@@ -458,24 +458,33 @@ private int
Github user sihuazhou commented on a diff in the pull request:
https://github.com/apache/flink/pull/6062#discussion_r190517336
--- Diff:
flink-streaming-java/src/main/java/org/apache/flink/streaming/api/operators/InternalTimer.java
---
@@ -23,8 +23,8 @@
/**
* Internal
Github user sihuazhou commented on the issue:
https://github.com/apache/flink/pull/6062
@StefanRRichter Thanks for your reply, I think I should also have a look at
https://github.com/apache/flink/pull/3359 to supplement my knowledge on this.
Looking forward your design document!
---
Github user sihuazhou commented on the issue:
https://github.com/apache/flink/pull/6062
Hi @StefanRRichter could I ask one more thing that I'm very interested in?
From this PR I can feel that the `InternalTimerHeap` is really like a
`HeapState` which scoped to the key group(not per
Github user sihuazhou commented on the issue:
https://github.com/apache/flink/pull/5847
cc @tillrohrmann could you please take a look at this?
---
1 - 100 of 536 matches
Mail list logo
