[improve][broker] Reduce the CPU pressure from the transaction buffer in rolling restarts #23062
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
31d847c to
79430fc
Compare
Contributor
Author
|
This improvement for the reader side can be verified by the following example. public class LoadTopicTest extends ProducerConsumerBase {
@BeforeClass
@Override
protected void setup() throws Exception {
conf.setDefaultNumberOfNamespaceBundles(1);
conf.setNumTransactionReplayThreadPoolSize(1);
conf.setTransactionCoordinatorEnabled(true);
super.isTcpLookup = true;
super.internalSetup();
super.producerBaseSetup();
}
@AfterClass
@Override
protected void cleanup() throws Exception {
super.internalCleanup();
}
@Test
public void test() throws Exception {
final var topic = "test";
admin.topics().createPartitionedTopic(topic, 5);
@Cleanup final var producer = pulsarClient.newProducer().topic(topic + "-partition-0").create();
Thread.sleep(1000);
}
}Save the outputs into Even though the After this change, the logs become |
Contributor
Author
|
Mark it as draft to avoid repeated creation of reader. It's better to maintain a long live reader. |
…ders and writers in rolling restarts
### Motivation
During the rolling restarts, the namespace bundle ownerships will
change. Assuming there is a producer created on a single topic, and the
ownership was transferred to the new broker. Assuming the namespace
bundle has N topics and the namespace is `tenant/ns`,
1. All N topics in the same bundle of that topic will be loaded.
2. For each topic, the managed ledger will be initialized, when the
transaction coordinator is enabled, a `TopicTransactionBuffer` will
be created.
2.1 A Pulsar producer will be created on
`tenant/ns/__transaction_buffer_snapshot` concurrently.
2.2 A Pulsar reader will be created on
`tenant/ns/__transaction_buffer_snapshot` concurrently.
3. Once all N readers are created, the owner of the snapshot topic will
start dispatching messages to N readers. Each dispatcher will read
messages from BookKeeper concurrently and might fail with too many
requests error because BK can only have
`maxPendingReadRequestsPerThread` pending read requests (default: 10000).
We have a `numTransactionReplayThreadPoolSize` config to limit the
concurrency of transaction snapshot readers. However, it only limits the
read loop. For example, if it's configured with 1, only 1 reader could
read messages at the same time. However, N readers will be created
concurrently. Each when one of these reader explicitly calls `readNext`,
all N dispatchers at brokers side will dispatch messages to N readers.
The behaviors above brings much CPU pressure on the owner broker,
especially for a small cluster with only two brokers.
### Modifications
- Synchronize the reader creation, read loop and the following process
on its result. Maintain only one reader for each namespace.
9378fa9 to
ffcc578
Compare
15 tasks
Contributor
|
@liangyepianzhou @congbobo184 @gaoran10 Please take a look |
ef22430 to
3b192a5
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #23062 +/- ##
============================================
- Coverage 73.57% 73.47% -0.11%
- Complexity 32624 33530 +906
============================================
Files 1877 1919 +42
Lines 139502 144086 +4584
Branches 15299 15741 +442
============================================
+ Hits 102638 105860 +3222
- Misses 28908 30101 +1193
- Partials 7956 8125 +169
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Contributor
Author
|
Merge it first. |
nikhil-ctds
pushed a commit
to datastax/pulsar
that referenced
this pull request
Aug 8, 2024
… in rolling restarts (apache#23062) (cherry picked from commit 40c8c23) (cherry picked from commit 53fb549)
srinath-ctds
pushed a commit
to datastax/pulsar
that referenced
this pull request
Aug 12, 2024
… in rolling restarts (apache#23062) (cherry picked from commit 40c8c23) (cherry picked from commit 53fb549)
hanmz
pushed a commit
to hanmz/pulsar
that referenced
this pull request
Feb 12, 2025
… in rolling restarts (apache#23062)
4 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation
During the rolling restarts, the namespace bundle ownerships will change. Assuming there is a producer created on a single topic, and the ownership was transferred to the new broker. Assuming the namespace bundle has N topics and the namespace is
tenant/ns,TopicTransactionBufferwill be created. A Pulsar reader will be created ontenant/ns/__transaction_buffer_snapshotconcurrently.maxPendingReadRequestsPerThreadpending read requests (default: 10000).We have a
numTransactionReplayThreadPoolSizeconfig to limit the concurrency of transaction snapshot readers. However, it only limits the read loop. For example, if it's configured with 1, only 1 reader could read messages at the same time. However, N readers will be created concurrently. Each when one of these reader explicitly callsreadNext, all N dispatchers at brokers side will dispatch messages to N readers.The behaviors above brings much CPU pressure on the owner broker, especially for a small cluster with only two brokers.
Modifications
Synchronize the reader creation, read loop and the following process on its result. Maintain only one reader for each namespace. The reader is now not closed unless there is no snapshot read request in 1 minute.
Documentation
docdoc-requireddoc-not-neededdoc-completeMatching PR in forked repository
PR in forked repository: BewareMyPower#32