Savonitar commented on code in PR #28639:
URL: https://github.com/apache/flink/pull/28639#discussion_r3536780852
##########
flink-runtime/src/main/java/org/apache/flink/runtime/resourcemanager/ResourceManager.java:
##########
@@ -427,6 +430,14 @@ public CompletableFuture<RegistrationResponse>
registerJobMaster(
jobMasterIdFuture,
(JobMasterGateway jobMasterGateway, JobMasterId
leadingJobMasterId) -> {
if (Objects.equals(leadingJobMasterId,
jobMasterId)) {
+ // Register with the delegation token
manager first; a
+ // provider failure rejects this
registration so the job
+ // never starts without the tokens it
requires.
+ try {
+
delegationTokenManager.registerJob(jobId, jobConfiguration);
+ } catch (Exception e) {
+ return new
RegistrationResponse.Failure(e);
Review Comment:
> oncall guys
I'm something of an on-call guy myself...
Yes, it **was** logged in
https://github.com/apache/flink/pull/28639/changes#diff-d32f89982ba30269ab9438d993d9deb3b29d046792b6cc5ea35a277eaacfb287R629
(flink-runtime/src/main/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManager.java
registerJob()
```
LOG.error("Failed to register job {}", jobId, e);
```
And moreover, we already had the failing **provider** in stack trace. The
rollback path has its own `ERROR log too ("Failed to roll back registration of
job {}")`.
Update to: ^ Since then the failure surfaces improved further: the logs now
name the failing provider directly (`Failed to register job {} for provider
{}`,
https://github.com/apache/flink/commit/62b931d7ac7124fdfa67ad20d0bfd2b82730ceaf),
and the JobMaster gets the failure back wrapped as a FlinkException naming the
job and the delegation token manager in the RegistrationResponse
(https://github.com/apache/flink/commit/3fd636b974d876a277390821a4857e9014ae891f).
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
