Skip to content

Conversation

@lordcheng10
Copy link
Contributor

@lordcheng10 lordcheng10 commented Oct 17, 2022

Motivation

image

In PR: https://github.com//pull/17273, when the read position is updated, the ManagedLedgerImpl#onCursorReadPositionUpdated method will be called, and finally ManagedCursorContainer#cursorUpdated will be called, thus making the slowestReaderPosition change:

void setReadPosition(Position newReadPositionInt) {
checkArgument(newReadPositionInt instanceof PositionImpl);
if (this.markDeletePosition == null
|| ((PositionImpl) newReadPositionInt).compareTo(this.markDeletePosition) > 0) {
this.readPosition = (PositionImpl) newReadPositionInt;
ledger.onCursorReadPositionUpdated(this, newReadPositionInt);
}

Therefore, in testActiveAndInActiveConsumerEntryCacheBehavior, as long as the server pushes data to the client, the slowestReaderPosition will change.

So here the receive queue of the slower-subscriber is set to 1 to prevent the slowestReaderPosition from changing even if the subscription is not consumed.

Documentation

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

Matching PR in forked repository

PR in forked repository: lordcheng10#31

@lordcheng10
Copy link
Contributor Author

@codelipenghui @Technoboy- @eolivelli @AnonHxy PTAL,thanks!

@codecov-commenter
Copy link

codecov-commenter commented Oct 18, 2022

Codecov Report

Merging #18075 (358c5db) into master (6c65ca0) will increase coverage by 12.61%.
The diff coverage is 78.19%.

Impacted file tree graph

@@              Coverage Diff              @@
##             master   #18075       +/-   ##
=============================================
+ Coverage     34.91%   47.52%   +12.61%     
- Complexity     5707    18023    +12316     
=============================================
  Files           607     1574      +967     
  Lines         53396   128320    +74924     
  Branches       5712    14116     +8404     
=============================================
+ Hits          18644    60987    +42343     
- Misses        32119    61122    +29003     
- Partials       2633     6211     +3578     
Flag Coverage Δ
unittests 47.52% <78.19%> (+12.61%) ⬆️

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

Impacted Files Coverage Δ
...org/apache/pulsar/broker/ServiceConfiguration.java 52.07% <ø> (ø)
.../service/SystemTopicBasedTopicPoliciesService.java 62.97% <0.00%> (+11.38%) ⬆️
...g/apache/pulsar/compaction/CompactedTopicImpl.java 73.57% <0.00%> (+62.85%) ⬆️
...va/org/apache/pulsar/client/impl/ConsumerBase.java 21.95% <0.00%> (ø)
...he/pulsar/functions/worker/rest/api/SinksImpl.java 36.82% <50.00%> (ø)
...apache/pulsar/proxy/server/DirectProxyHandler.java 63.63% <50.00%> (ø)
...ulsar/functions/worker/rest/api/ComponentImpl.java 25.26% <57.14%> (ø)
...ava/org/apache/pulsar/common/util/PortManager.java 60.00% <60.00%> (ø)
...broker/delayed/InMemoryDelayedDeliveryTracker.java 65.00% <75.00%> (+65.00%) ⬆️
...che/bookkeeper/mledger/impl/ManagedLedgerImpl.java 71.33% <80.00%> (ø)
... and 1156 more

@lordcheng10
Copy link
Contributor Author

/pulsarbot run-failure-checks

Copy link
Contributor

@aloyszhang aloyszhang left a comment

Choose a reason for hiding this comment

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

LGTM

@lordcheng10
Copy link
Contributor Author

@tisonkun PTAL,thanks!

@lordcheng10
Copy link
Contributor Author

/pulsarbot run-failure-checks

1 similar comment
@lordcheng10
Copy link
Contributor Author

/pulsarbot run-failure-checks

@aloyszhang aloyszhang merged commit b3733dd into apache:master Oct 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/test doc-not-needed Your PR changes do not impact docs ready-to-test

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants