Skip to content

[improve][broker] Add javadoc for PulsarDecoder handle* method parameter lifecycle - #18998

Merged
lhotari merged 1 commit into
apache:masterfrom
lhotari:lh-add-javadoc-for-PulsarDecoder-handle-parameter-lifecycle
Dec 20, 2022
Merged

[improve][broker] Add javadoc for PulsarDecoder handle* method parameter lifecycle#18998
lhotari merged 1 commit into
apache:masterfrom
lhotari:lh-add-javadoc-for-PulsarDecoder-handle-parameter-lifecycle

Conversation

@lhotari

@lhotari lhotari commented Dec 20, 2022

Copy link
Copy Markdown
Member

Motivation

The lifecycle of the PulsarDecoder handle* method parameters isn't currently documented.

This PR adds this kind of paragraph:

Please be aware that the decoded protocol command instance passed to a handle* method is cleared and reused for the next protocol command after the method completes. This is done in order to minimize object allocations for performance reasons. It is not allowed to retain a reference to the handle* method parameter command instance after the method returns. If you need to pass an instance of the command instance to another thread or retain a reference to it after the handle* method completes, you must make a deep copy of the command instance.

See #18987 or #10215 for a sample issues where a related problem was fixed.

Modifications

Add Javadoc to PulsarDecoder and reference it in the implementation classes.

Documentation

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

Matching PR in forked repository

PR in forked repository: lhotari#115

@lhotari lhotari added this to the 2.12.0 milestone Dec 20, 2022
@lhotari lhotari self-assigned this Dec 20, 2022
@github-actions github-actions Bot added the doc Your PR contains doc changes, no matter whether the changes are in markdown or code files. label Dec 20, 2022
@lhotari
lhotari force-pushed the lh-add-javadoc-for-PulsarDecoder-handle-parameter-lifecycle branch from 52ec790 to ba09cc2 Compare December 20, 2022 09:29
@lhotari
lhotari requested a review from cbornet December 20, 2022 09:30
@lhotari
lhotari force-pushed the lh-add-javadoc-for-PulsarDecoder-handle-parameter-lifecycle branch from ba09cc2 to 36dbe20 Compare December 20, 2022 09:31
@lhotari

lhotari commented Dec 20, 2022

Copy link
Copy Markdown
Member Author

/pulsarbot rerun-failure-checks

@codecov-commenter

codecov-commenter commented Dec 20, 2022

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 46.48%. Comparing base (feb3cb4) to head (b272661).
⚠️ Report is 3109 commits behind head on master.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #18998      +/-   ##
============================================
+ Coverage     46.35%   46.48%   +0.12%     
- Complexity     8939    10475    +1536     
============================================
  Files           597      706     +109     
  Lines         56858    69005   +12147     
  Branches       5905     7391    +1486     
============================================
+ Hits          26357    32077    +5720     
- Misses        27616    33313    +5697     
- Partials       2885     3615     +730     
Flag Coverage Δ
unittests 46.48% <ø> (+0.12%) ⬆️

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

Files with missing lines Coverage Δ
...va/org/apache/pulsar/broker/service/ServerCnx.java 49.66% <ø> (+0.68%) ⬆️
.../java/org/apache/pulsar/client/impl/ClientCnx.java 29.97% <ø> (ø)
...org/apache/pulsar/proxy/server/ProxyClientCnx.java 46.51% <ø> (ø)
...rg/apache/pulsar/proxy/server/ProxyConnection.java 55.21% <ø> (ø)

... and 159 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.

@michaeljmarshall michaeljmarshall left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/broker doc Your PR contains doc changes, no matter whether the changes are in markdown or code files. ready-to-test

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants