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

[improve][fn] Improve closing of producers in Pulsar Functions ProducerCache invalidation #23734

Merged

Conversation

lhotari
Copy link
Member

@lhotari lhotari commented Dec 16, 2024

Motivation

  • The log messages are verbose when PulsarClientException.AlreadyClosedException is returned from closeAsync.
    • Don't log the AlreadyClosedException stack trace
  • There are concerns that producers could be left in the cache in exceptional cases. Therefore add handling to handle possible runtime exceptions in flushAsync and closeAsync calls.

Modifications

  • improve ProducerCache logic
  • add tests

Documentation

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

@codecov-commenter
Copy link

codecov-commenter commented Dec 16, 2024

Codecov Report

Attention: Patch coverage is 80.00000% with 3 lines in your changes missing coverage. Please review.

Project coverage is 74.42%. Comparing base (bbc6224) to head (b462241).
Report is 794 commits behind head on master.

Files with missing lines Patch % Lines
...pache/pulsar/functions/instance/ProducerCache.java 80.00% 2 Missing and 1 partial ⚠️
Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #23734      +/-   ##
============================================
+ Coverage     73.57%   74.42%   +0.85%     
- Complexity    32624    35111    +2487     
============================================
  Files          1877     1945      +68     
  Lines        139502   147518    +8016     
  Branches      15299    16282     +983     
============================================
+ Hits         102638   109794    +7156     
- Misses        28908    29252     +344     
- Partials       7956     8472     +516     
Flag Coverage Δ
inttests 27.29% <0.00%> (+2.71%) ⬆️
systests 24.39% <26.66%> (+0.06%) ⬆️
unittests 73.79% <80.00%> (+0.95%) ⬆️

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

Files with missing lines Coverage Δ
...pache/pulsar/functions/instance/ProducerCache.java 85.00% <80.00%> (ø)

... and 672 files with indirect coverage changes

@lhotari lhotari merged commit 9f046a5 into apache:master Dec 17, 2024
55 of 56 checks passed
lhotari added a commit that referenced this pull request Dec 18, 2024
…erCache invalidation (#23734)

(cherry picked from commit 9f046a5)
lhotari added a commit that referenced this pull request Dec 18, 2024
…erCache invalidation (#23734)

(cherry picked from commit 9f046a5)
lhotari added a commit that referenced this pull request Dec 18, 2024
…erCache invalidation (#23734)

(cherry picked from commit 9f046a5)
nikhil-ctds pushed a commit to datastax/pulsar that referenced this pull request Dec 19, 2024
…erCache invalidation (apache#23734)

(cherry picked from commit 9f046a5)
(cherry picked from commit 6b9e9ac)
nikhil-ctds pushed a commit to datastax/pulsar that referenced this pull request Dec 21, 2024
…erCache invalidation (apache#23734)

(cherry picked from commit 9f046a5)
(cherry picked from commit 6b9e9ac)
srinath-ctds pushed a commit to datastax/pulsar that referenced this pull request Dec 23, 2024
…erCache invalidation (apache#23734)

(cherry picked from commit 9f046a5)
(cherry picked from commit 6b9e9ac)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants