Dark Mode

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

[fix][fn] complete flushAsync before closeAsync in ProducerCache and wait for completion in closing the cache#25140

Merged
Technoboy- merged 2 commits intoapache:masterfrom
dontgitit:ilya/fix-producer-cache-flush-close
Jan 14, 2026
Merged

[fix][fn] complete flushAsync before closeAsync in ProducerCache and wait for completion in closing the cache#25140
Technoboy- merged 2 commits intoapache:masterfrom
dontgitit:ilya/fix-producer-cache-flush-close

Conversation

Copy link
Contributor

dontgitit commented Jan 14, 2026 *
edited
Loading

Fixes issue discussed on Slack

Motivation

While trying to implement a very similar component, I was poking around the ProducerCache and noticed that closeAsync was invoked without waiting for flushAsync to complete. I believe this could lead to correctness issues.

Modifications

I've simplified the code to directly call flushAsync, rather than start a future that calls it.

Verifying this change

  • Make sure that the change passes the CI checks.

This change attempted to add tests, but they currently fail because the existing test harness makes false assumptions / the code it's testing doesn't actually work (specifically, ProducerCache.close doesn't actually wait for cache removal listener futures).

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

If the box was checked, please highlight the changes

  • Dependencies (add or upgrade a dependency)
  • The public API
  • The schema
  • The default values of configurations
  • The threading model
  • The binary protocol
  • The REST endpoints
  • The admin CLI options
  • The metrics
  • Anything that affects deployment

Documentation

  • doc
  • doc-required
  • doc-not-needed
  • doc-complete

Matching PR in forked repository

PR in forked repository: dontgitit#1

github-actions bot added the doc-not-needed Your PR changes do not impact docs label Jan 14, 2026
lhotari approved these changes Jan 14, 2026
Copy link
Member

lhotari left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the good work @dontgitit. I added a commit to fix the test to implement the graceful closing of the cache based one of the workarounds mentioned in the issue that you pointed out in the slack discussion.

dontgitit reacted with heart emoji
lhotari changed the title [fix][functions] make sure flushAsync completes before calling closeAsync in ProducerCache [fix][fn] complete flushAsync before closeAsync in ProducerCache and wait for completion in closing the cache Jan 14, 2026
Copy link

codecov-commenter commented Jan 14, 2026

Codecov Report

Patch coverage is 85.71429% with 2 lines in your changes missing coverage. Please review.
Project coverage is 74.50%. Comparing base (1fcdf8b) to head (71b0b35).
Report is 2 commits behind head on master.

Files with missing lines Patch % Lines
...pache/pulsar/functions/instance/ProducerCache.java 85.71% 2 Missing
Additional details and impacted files

@@ Coverage Diff @@
## master #25140 +/- ##
=============================================
+ Coverage 30.69% 74.50% +43.80%
- Complexity 51 33698 +33647
=============================================
Files 1840 1899 +59
Lines 145551 149745 +4194
Branches 16924 17411 +487
=============================================
+ Hits 44679 111569 +66890
+ Misses 93910 29300 -64610
- Partials 6962 8876 +1914
Flag Coverage D
inttests 26.33% <0.00%> (+0.22%)
systests 23.10% <85.71%> (+0.08%)
unittests 74.04% <85.71%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage D
...pache/pulsar/functions/instance/ProducerCache.java 80.00% <85.71%> (+15.48%)

... and 1492 files with indirect coverage changes

New features to boost your workflow:
  • Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Technoboy- assigned dontgitit Jan 14, 2026
Technoboy- added this to the 4.2.0 milestone Jan 14, 2026
Technoboy- approved these changes Jan 14, 2026
Technoboy- merged commit 21819c6 into apache:master Jan 14, 2026
104 of 106 checks passed
dontgitit deleted the ilya/fix-producer-cache-flush-close branch January 14, 2026 20:52
lhotari added a commit that referenced this pull request Jan 16, 2026
...wait for completion in closing the cache (#25140)

Co-authored-by: Lari Hotari
(cherry picked from commit 21819c6)
lhotari added the cherry-picked/branch-3.0 label Jan 16, 2026
lhotari added a commit that referenced this pull request Jan 16, 2026
...wait for completion in closing the cache (#25140)

Co-authored-by: Lari Hotari
(cherry picked from commit 21819c6)
lhotari added the cherry-picked/branch-4.0 label Jan 16, 2026
lhotari added a commit that referenced this pull request Jan 16, 2026
...wait for completion in closing the cache (#25140)

Co-authored-by: Lari Hotari
(cherry picked from commit 21819c6)
lhotari added the cherry-picked/branch-4.1 label Jan 16, 2026
manas-ctds pushed a commit to datastax/pulsar that referenced this pull request Jan 19, 2026
...wait for completion in closing the cache (apache#25140)

Co-authored-by: Lari Hotari
(cherry picked from commit 21819c6)
(cherry picked from commit c22f3d5)
priyanshu-ctds pushed a commit to datastax/pulsar that referenced this pull request Jan 20, 2026
...wait for completion in closing the cache (apache#25140)

Co-authored-by: Lari Hotari
(cherry picked from commit 21819c6)
(cherry picked from commit 00def71)
manas-ctds pushed a commit to datastax/pulsar that referenced this pull request Jan 20, 2026
...wait for completion in closing the cache (apache#25140)

Co-authored-by: Lari Hotari
(cherry picked from commit 21819c6)
(cherry picked from commit c22f3d5)
srinath-ctds pushed a commit to datastax/pulsar that referenced this pull request Jan 20, 2026
...wait for completion in closing the cache (apache#25140)

Co-authored-by: Lari Hotari
(cherry picked from commit 21819c6)
(cherry picked from commit 00def71)
srinath-ctds pushed a commit to datastax/pulsar that referenced this pull request Jan 28, 2026
...wait for completion in closing the cache (apache#25140)

Co-authored-by: Lari Hotari
(cherry picked from commit 21819c6)
(cherry picked from commit c22f3d5)
Rutuja-IBM pushed a commit to datastax/pulsar that referenced this pull request Feb 9, 2026
...wait for completion in closing the cache (apache#25140)

Co-authored-by: Lari Hotari
(cherry picked from commit 21819c6)
(cherry picked from commit c22f3d5)
Rutuja-IBM pushed a commit to datastax/pulsar that referenced this pull request Feb 9, 2026
...wait for completion in closing the cache (apache#25140)

Co-authored-by: Lari Hotari
(cherry picked from commit 21819c6)
(cherry picked from commit c22f3d5)
Rutuja-IBM pushed a commit to datastax/pulsar that referenced this pull request Feb 12, 2026
...wait for completion in closing the cache (apache#25140)

Co-authored-by: Lari Hotari
(cherry picked from commit 21819c6)
(cherry picked from commit c22f3d5)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

lhotari lhotari approved these changes

Technoboy- Technoboy- approved these changes

Assignees

dontgitit

Projects

None yet

Milestone

4.2.0

Development

Successfully merging this pull request may close these issues.

4 participants