Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FLINK-26494][runtime] Cleanup does not reveal exceptions #18987

Merged
merged 3 commits into from Mar 10, 2022

Conversation

XComp
Copy link
Contributor

@XComp XComp commented Mar 6, 2022

What is the purpose of the change

The retryable cleanup works but does not reveal the actual error which makes it harder to identify the error. This PR fixes it.

Brief change log

  • DefaultResourceCleaner.Builder method signatures for adding cleanups are extended to also require a label that is passed. The label should be unique to make each cleanup identifyable in the logs.

Verifying this change

  • A test was added to check the uniqueness of the label
  • The logging was verified manually by running the DefaultResourceCleanerTest methods with logging enabled

Does this pull request potentially affect one of the following parts:

  • Dependencies (does it add or upgrade a dependency): no
  • The public API, i.e., is any changed class annotated with @Public(Evolving): no
  • The serializers: no
  • The runtime per-record code paths (performance sensitive): no
  • Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Kubernetes/Yarn, ZooKeeper: no
  • The S3 file system connector: no

Documentation

  • Does this pull request introduce a new feature? no
  • If yes, how is the feature documented? JavaDoc

@flinkbot
Copy link
Collaborator

flinkbot commented Mar 6, 2022

CI report:

Bot commands The @flinkbot bot supports the following commands:
  • @flinkbot run azure re-run the last Azure build

@zentol zentol self-assigned this Mar 9, 2022
@XComp
Copy link
Contributor Author

XComp commented Mar 9, 2022

Thanks for your review, @zentol . I addressed your comments. PTAL

@XComp
Copy link
Contributor Author

XComp commented Mar 10, 2022

Updated the commit messages and rebased to the most recent version of master to verify that there are no conflicts. I ran FileSystemJobResultStoreTestInternal and FileSystemJobResultStoreContractTest to verify it locally.

@XComp XComp merged commit c5352fc into apache:master Mar 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants