pubsublite: Reduce commit logspam

[dpcollins-google]

---

Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:

• Choose reviewer(s) and mention them in a comment (R: @username).
• Mention the appropriate issue in your description (for example: addresses #123), if applicable. This will automatically add a link to the pull request in the issue. If you would like the issue to automatically close on merging the pull request, comment fixes #<ISSUE NUMBER> instead.
• Update CHANGES.md with noteworthy changes.
• If this contribution is large, please file an Apache Individual Contributor License Agreement.

See the Contributor Guide for more tips on how to make review process smoother.

To check the build health, please visit https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md

GitHub Actions Tests Status (on master branch)

See CI.md for more information about GitHub Actions CI.

[github-actions]

Assigning reviewers. If you would like to opt out of this review, comment assign to next reviewer:

R: @kennknowles for label java.
R: @Abacn for label io.

Available commands:

• stop reviewer notifications - opt out of the automated review tooling
• remind me after tests pass - tag the comment author after tests pass
• waiting on author - shift the attention set back to the author (any comment or push by the author will return the attention set to the reviewers)

The PR bot will only process comments in the main thread (not review comments).

[Abacn]

Thanks for the work. Would you mind providing more context in the description or is there a linked issue? What kind of "log spam" we current have?

[dpcollins-google]

There is not a linked issue. Whenever dataflow scales up or down, it closes the reader while there are still outstanding checkpointmarks that will be committed asynchronously and fail, logging a bunch of warnings. CheckpointMark has no lifecycle methods- hence the only way to ensure the committer lives long enough for all the CheckpointMarks to be used (while also not requiring that they are used) is with finalize.

[github-actions]

Reminder, please take a look at this pr: @kennknowles @Abacn

[github-actions]

Assigning new set of reviewers because Pr has gone too long without review. If you would like to opt out of this review, comment assign to next reviewer:

R: @robertwb for label java.
R: @johnjcasey for label io.

Available commands:

• stop reviewer notifications - opt out of the automated review tooling
• remind me after tests pass - tag the comment author after tests pass
• waiting on author - shift the attention set back to the author (any comment or push by the author will return the attention set to the reviewers)

[Abacn]

waiting on author

[dpcollins-google]

applied minimal suggestion- please merge if there's nothing else?

[dpcollins-google]

Run Java PostCommit

[johnjcasey]

This is somewhat beyond my understanding.

I'd prefer to not use a depreciated method if possible, but for the release I believe this is ok.

Was there a reason you can't simply add a finalize() method to CheckpointMarkImpl directly, avoiding this indirection?

[bvolpato]

LGTM (non-committer).

Ideally we could put a TODO to remember to refactor it to use java.lang.ref.Cleaner instead of finalize() if we ever move to JDK 9+.

[dpcollins-google]

This is somewhat beyond my understanding.

I'd prefer to not use a depreciated method if possible, but for the release I believe this is ok.

Was there a reason you can't simply add a finalize() method to CheckpointMarkImpl directly, avoiding this indirection?

Yes, we only want to close when no checkpointMarks exist that are using this, not when each goes out of scope.