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/14 07:04:36 UTC
[GitHub] [flink] zentol opened a new pull request, #19469: [FLINK-27222][coordination] Decouple last (al)location from execution history
zentol opened a new pull request, #19469:
URL: https://github.com/apache/flink/pull/19469
Prevents loss of information that is critical to local recovery due to evictions from the execution history.
Previously a fast restart loop on TM loss could result in local recovery never kicking in because all the information was lost.
Instead we store the last (al)location in the vertex.
--
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] XComp commented on a diff in pull request #19469: [FLINK-27222][coordination] Decouple last (al)location from execution history
Posted by GitBox <gi...@apache.org>.
XComp commented on code in PR #19469:
URL: https://github.com/apache/flink/pull/19469#discussion_r850363307
##########
flink-runtime/src/main/java/org/apache/flink/runtime/executiongraph/ExecutionVertex.java:
##########
@@ -313,11 +296,15 @@ private <T> Optional<T> getLatestPriorProperty(Function<ArchivedExecution, T> ex
* @return The latest prior execution location, or null, if there is none, yet.
*/
public Optional<TaskManagerLocation> findLatestPriorLocation() {
- return getLatestPriorProperty(ArchivedExecution::getAssignedResourceLocation);
+ return Optional.ofNullable(lastAssignedLocation);
+ }
+
+ void setLatestPriorAllocation(AllocationID lastAssignedAllocationID) {
Review Comment:
I thought about it once more: I think it makes sense to merge `setLatestPriorAllocation` and `setLatestPrioirLocation` into something like `setLatestPriorSlotAssignment(TaskManagerLocation, AllocationID)` because both are closely related...
##########
flink-runtime/src/main/java/org/apache/flink/runtime/executiongraph/ExecutionVertex.java:
##########
@@ -313,11 +296,15 @@ private <T> Optional<T> getLatestPriorProperty(Function<ArchivedExecution, T> ex
* @return The latest prior execution location, or null, if there is none, yet.
*/
public Optional<TaskManagerLocation> findLatestPriorLocation() {
Review Comment:
Can we rephrase these `findLatestPrior*` and `setLatestPrior*` methods. Because we're setting it now when assigning the resource it means that we actually return the latest assignment and not the latest prior assignment...
--
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] flinkbot commented on pull request #19469: [FLINK-27222][coordination] Decouple last (al)location from execution history
Posted by GitBox <gi...@apache.org>.
flinkbot commented on PR #19469:
URL: https://github.com/apache/flink/pull/19469#issuecomment-1098783347
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "3534ec3e79390fbe8e12c8d5373b126881d9a2eb",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "3534ec3e79390fbe8e12c8d5373b126881d9a2eb",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 3534ec3e79390fbe8e12c8d5373b126881d9a2eb UNKNOWN
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@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.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] XComp commented on a diff in pull request #19469: [FLINK-27222][coordination] Decouple last (al)location from execution history
Posted by GitBox <gi...@apache.org>.
XComp commented on code in PR #19469:
URL: https://github.com/apache/flink/pull/19469#discussion_r850405831
##########
flink-runtime/src/main/java/org/apache/flink/runtime/executiongraph/ExecutionVertex.java:
##########
@@ -313,11 +296,15 @@ private <T> Optional<T> getLatestPriorProperty(Function<ArchivedExecution, T> ex
* @return The latest prior execution location, or null, if there is none, yet.
*/
public Optional<TaskManagerLocation> findLatestPriorLocation() {
Review Comment:
it's not about the set and find but the `LatestPrior` substring that is not valid anymore
--
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] zentol merged pull request #19469: [FLINK-27222][coordination] Decouple last (al)location from execution history
Posted by GitBox <gi...@apache.org>.
zentol merged PR #19469:
URL: https://github.com/apache/flink/pull/19469
--
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] zentol commented on a diff in pull request #19469: [FLINK-27222][coordination] Decouple last (al)location from execution history
Posted by GitBox <gi...@apache.org>.
zentol commented on code in PR #19469:
URL: https://github.com/apache/flink/pull/19469#discussion_r850400271
##########
flink-runtime/src/main/java/org/apache/flink/runtime/executiongraph/ExecutionVertex.java:
##########
@@ -313,11 +296,15 @@ private <T> Optional<T> getLatestPriorProperty(Function<ArchivedExecution, T> ex
* @return The latest prior execution location, or null, if there is none, yet.
*/
public Optional<TaskManagerLocation> findLatestPriorLocation() {
Review Comment:
Had the same thoughts, but one could argue that whether it is set or find through some fancy algorithm is an implementation detail, so I reverted it.
##########
flink-runtime/src/main/java/org/apache/flink/runtime/executiongraph/ExecutionVertex.java:
##########
@@ -313,11 +296,15 @@ private <T> Optional<T> getLatestPriorProperty(Function<ArchivedExecution, T> ex
* @return The latest prior execution location, or null, if there is none, yet.
*/
public Optional<TaskManagerLocation> findLatestPriorLocation() {
Review Comment:
Had the same thoughts, but one could argue that whether it is set or found through some fancy algorithm is an implementation detail, so I reverted it.
--
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] zentol commented on a diff in pull request #19469: [FLINK-27222][coordination] Decouple last (al)location from execution history
Posted by GitBox <gi...@apache.org>.
zentol commented on code in PR #19469:
URL: https://github.com/apache/flink/pull/19469#discussion_r850410404
##########
flink-runtime/src/main/java/org/apache/flink/runtime/executiongraph/ExecutionVertex.java:
##########
@@ -313,11 +296,15 @@ private <T> Optional<T> getLatestPriorProperty(Function<ArchivedExecution, T> ex
* @return The latest prior execution location, or null, if there is none, yet.
*/
public Optional<TaskManagerLocation> findLatestPriorLocation() {
Review Comment:
ah
--
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