[GitHub] [flink] XComp commented on pull request #22422: [FLINK-31838][runtime] Moves thread handling for leader event listener calls from DefaultMultipleComponentLeaderElectionService into DefaultLea
XComp commented on PR #22422: URL: https://github.com/apache/flink/pull/22422#issuecomment-1535006196 Did a final rebase after PR #22517 was merged to `master` to run the test suite with the proper `DirectExecutorService` implementation. -- 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-unsubscr...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [flink] XComp commented on pull request #22422: [FLINK-31838][runtime] Moves thread handling for leader event listener calls from DefaultMultipleComponentLeaderElectionService into DefaultLea
XComp commented on PR #22422: URL: https://github.com/apache/flink/pull/22422#issuecomment-1534632811 Squashed the commits together again to prepare the PR for the merge. -- 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-unsubscr...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [flink] XComp commented on pull request #22422: [FLINK-31838][runtime] Moves thread handling for leader event listener calls from DefaultMultipleComponentLeaderElectionService into DefaultLea
XComp commented on PR #22422: URL: https://github.com/apache/flink/pull/22422#issuecomment-1534630265 I had to rebase because of FLINK-31962. -- 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-unsubscr...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [flink] XComp commented on pull request #22422: [FLINK-31838][runtime] Moves thread handling for leader event listener calls from DefaultMultipleComponentLeaderElectionService into DefaultLea
XComp commented on PR #22422: URL: https://github.com/apache/flink/pull/22422#issuecomment-1534245422 There's a test failure that popped up in [CI](https://dev.azure.com/apache-flink/apache-flink/_build/results?buildId=48461=logs=af184cdd-c6d8-5084-0b69-7e9c67b35f7a=0f3adb59-eefa-51c6-2858-3654d9e0749d=15266): Due to the fact that the drivers could trigger the event handling methods (i.e. `onRevokeLeadership` and `onGrantLeadership`) in a separate thread, it could happen that we stop the `DefaultLeaderElectionService` but there's still a event handling call in some thread queue that's triggered after the service is stopped (and consequently, the service's `leadershipOperationExecutor`). That will lead to a `RejectedExecutionException` because we're not checking whether the service is still running before putting the task in the executor's pool. We actually have unit tests for covering this issue. That just didn't fire because the `DirectExecutorService` implementation doesn't follow the `ExecutorService`'s interface contract. I created FLINK-31995 to cover this issue. -- 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-unsubscr...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [flink] XComp commented on pull request #22422: [FLINK-31838][runtime] Moves thread handling for leader event listener calls from DefaultMultipleComponentLeaderElectionService into DefaultLea
XComp commented on PR #22422: URL: https://github.com/apache/flink/pull/22422#issuecomment-1521352662 fixed test failure and rebased branch. -- 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-unsubscr...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [flink] XComp commented on pull request #22422: [FLINK-31838][runtime] Moves thread handling for leader event listener calls from DefaultMultipleComponentLeaderElectionService into DefaultLea
XComp commented on PR #22422: URL: https://github.com/apache/flink/pull/22422#issuecomment-1520425379 Thanks for the reviews. I squashed the commits in https://github.com/apache/flink/commit/e1997df38d2bd380804a1a1814d14b75db2493ed and rebased the branch in https://github.com/apache/flink/commit/97082098f891fb0b169538ca8445cf7810e660e2 -- 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-unsubscr...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [flink] XComp commented on pull request #22422: [FLINK-31838][runtime] Moves thread handling for leader event listener calls from DefaultMultipleComponentLeaderElectionService into DefaultLea
XComp commented on PR #22422: URL: https://github.com/apache/flink/pull/22422#issuecomment-1519947329 I reverted the changes that were done in `DefaultMultipleComponentLeaderElectionService` because they are not necessary to make the leader event handling run in a separate thread in `DefaultLeaderElectionService` (as figured out in [the comment above](https://github.com/apache/flink/pull/22422#discussion_r1175063876). `DefaultMultipleComponentLeaderElectionService` will be removed at the end of the FLINK-26522 refactoring anyway. I added a `@NotThreadSafe` annotation to the `LeaderElectionEventHandler` interface to acknowledge that the methods of that interface have to be called from the same thread. -- 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-unsubscr...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [flink] XComp commented on pull request #22422: [FLINK-31838][runtime] Moves thread handling for leader event listener calls from DefaultMultipleComponentLeaderElectionService into DefaultLea
XComp commented on PR #22422: URL: https://github.com/apache/flink/pull/22422#issuecomment-1516600026 Thanks @reswqa for your comments. I addressed them and push the commits. Looks like CI will remain green as well. Therefore, the PR is ready to be reviewed. I would go ahead and squash everything into reasonable chunks after the review is done. -- 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-unsubscr...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [flink] XComp commented on pull request #22422: [FLINK-31838][runtime] Moves thread handling for leader event listener calls from DefaultMultipleComponentLeaderElectionService into DefaultLea
XComp commented on PR #22422: URL: https://github.com/apache/flink/pull/22422#issuecomment-1514911066 I will reiterate over the `LeaderElectionEventHandler` interface tomorrow and address @reswqa 's comments. No need to review this PR, yet. Sorry for the poor interface design proposal. -- 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-unsubscr...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
