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

[run-tests] [improvement] Introduce the sync( API to ensure consistency on reads during critical metadata operation paths #20

Open
wants to merge 243 commits into
base: master
Choose a base branch
from

Conversation

eolivelli
Copy link
Owner

This PR is for running tests for upstream PR apache#18518.

)

AnonHxy and others added 29 commits November 18, 2022 10:13
…sage (apache#18511)

### Motivation

Currently, if there are duplicated messages whose chunk id is both 0, then it may result in allocating an unused buffer and may lead to the buffer memory leak.

### Modifications

* Only allocate the bytebuffer when there is no duplicated chunk message with chunk id 0.

Signed-off-by: Zike Yang <zike@apache.org>
Signed-off-by: Zixuan Liu <nodeces@gmail.com>
### Motivation

We already added the seek support for multi-topics consumer in apache#7518. But the note for seek method hasn't been updated.

### Modifications

* Update the doc for seek method in the consumer.
…ered when using batch receive and trigger timeout (apache#18478)
…ion (apache#18451)

Signed-off-by: Paul Gier <paul.gier@datastax.com>

Fixes apache#15819

The existing code calculates the pulsar_out_bytes_total and pulsar_out_messages_total per subscription metrics by adding the values from the currently connected consumers. This produces incorrect values as soon as one or more of the consumers disconnects from the subscription.

This changes these two metrics to directly use the subscription stats for these values, and match the output of `pulsar-admin topic stats`.

Signed-off-by: Paul Gier <paul.gier@datastax.com>

Fixes apache#15819

### Motivation

The prometheus metrics for pulsar_out_bytes_total and pulsar_out_messages_total should never decrease,
and they should match the output seen when using pulsar-admin.

### Modifications

Changed the calculation of pulsar_out_bytes_total and pulsar_out_messages_total to directly use the
subscription stats instead of calculating these values by summing the values of the currently connected
consumers.

### Verifying this change

- [X] Make sure that the change passes the CI checks.

Added a unit test to cover this case.

### 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
- [ ] Anything that affects deployment

### Documentation

<!-- DO NOT REMOVE THIS SECTION. CHECK THE PROPER BOX ONLY. -->

- [ ] `doc` <!-- Your PR contains doc changes. Please attach the local preview screenshots (run `sh start.sh` at `pulsar/site2/website`) to your PR description, or else your PR might not get merged. -->
- [ ] `doc-required` <!-- Your PR changes impact docs and you will update later -->
- [X] `doc-not-needed` <!-- Your PR changes do not impact docs -->
- [ ] `doc-complete` <!-- Docs have been already added -->

### Matching PR in forked repository

PR in forked repository: pgier#2
Co-authored-by: tison <wander4096@gmail.com>
Signed-off-by: Paul Gier <paul.gier@datastax.com>
…apache#18486)

### Motivation

apache#18454 fixed the potential message loss when a batched message is redelivered and one single message of the batch is added to the ACK tracker. However, it also leads to a potential message duplication, see the `testConsumerDedup` test modified by apache#18454.

The root cause is that single messages will still be passed into the `isDuplicated` method in `receiveIndividualMessagesFromBatch`. However, in this case, the `MessageId` is a `BatchedMessageIdImpl`, while the `MessageId` in `lastCumulativeAck` or `pendingIndividualAcks` are `MessageIdImpl` implementations.

### Modifications

Validate the class type in `isDuplicated` and convert a `BatchedMessageIdImpl` to `MessageIdImpl`. Then revert the unnecessary changes in apache#18454.

`ConsumerRedeliveryTest#testAckNotSent` is added to verify it works.

### TODO

The duplication could still happen when batch index ACK is enabled. Because even after the ACK tracker is flushed, if only parts of a batched message are not acknowledged, the whole batched message would still be redelivered. I will open another PR to fix it.

### Documentation

<!-- DO NOT REMOVE THIS SECTION. CHECK THE PROPER BOX ONLY. -->

- [ ] `doc` <!-- Your PR contains doc changes. Please attach the local preview screenshots (run `sh start.sh` at `pulsar/site2/website`) to your PR description, or else your PR might not get merged. -->
- [ ] `doc-required` <!-- Your PR changes impact docs and you will update later -->
- [x] `doc-not-needed` <!-- Your PR changes do not impact docs -->
- [ ] `doc-complete` <!-- Docs have been already added -->

### Matching PR in forked repository

PR in forked repository: BewareMyPower#8
tisonkun and others added 29 commits December 21, 2022 12:13
…pache#18996)

Co-authored-by: gavingaozhangmin <gavingaozhangmin@didiglobal.com>
…meterized string formatting (apache#18968)

Co-authored-by: Ali Ahmed <alia@splunk.com>
Co-authored-by: fengwenzhi <fengwenzhi.max@bigo.sg>
Co-authored-by: lushiji <lushiji@didiglobal.com>
Signed-off-by: tison <wander4096@gmail.com>
…for paritioned topic stats-internal (apache#19044)

Signed-off-by: ericsyh <ericshenyuhao@outlook.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.