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

[fix][flaky-test]NegativeAcksTest.testNegativeAcksWithBatchAckEnabled #17893

Conversation

poorbarcode
Copy link
Contributor

@poorbarcode poorbarcode commented Sep 29, 2022

Fixes: #16864

Motivation

I think it is a wrong configuration(ackTimeout 1s) when writing the code, the original design is set negativeAckRedeliveryDelay 1s

The process expects:

  • send 10 messages in one batch
    • submit a batch.
  • receive 10 messages, do negative acknowledge
  • after 1s, will trigger redelivery
  • receive 10 messages again

The real process:

  • send 1 message
    • Reach the batch time limit, and submit a batch. return msgId_1
  • send 9 messages in another batch
    • submit a batch. return msgId_2
  • receive 10 messages, do negative acknowledge
    • push the msgId_1 to negativeAcksTracker
    • push the msgId_2 to unAckedMessageTracker
  • after 1s, will trigger redelivery msgId_2 by unAckedMessageTracker
  • receive 9 messages( msgId_2 ) again
  • after 60s, will trigger redelivery msgId_1 by negativeAcksTracker. (High light) Test execution timeout!
  • receive 1 messages( msgId_1 ) again

Modifications

  • remove conf: ackTimeout
  • set negativeAckRedeliveryDelay 1s

Documentation

  • doc-not-needed

Matching PR in forked repository

PR in forked repository:

@congbobo184 congbobo184 added this to the 2.11.0 milestone Sep 30, 2022
@congbobo184 congbobo184 marked this pull request as draft September 30, 2022 02:22
@poorbarcode poorbarcode marked this pull request as ready for review September 30, 2022 02:23
recover timeout to `10s`. because I rerun it locally again, cost only 5S
@congbobo184 congbobo184 merged commit 85b1138 into apache:master Sep 30, 2022
@poorbarcode poorbarcode deleted the flaky/testNegativeAcksWithBatchAckEnabled branch October 12, 2022 09:11
lhotari pushed a commit that referenced this pull request Aug 9, 2024
…#17893)

Fixes: #16864

### Motivation

I think it is a wrong configuration(`ackTimeout 1s`) when writing the code, the original design is set `negativeAckRedeliveryDelay 1s`

The process expects:

- send 10 messages in one batch
  - submit a batch.
- receive 10 messages, do negative acknowledge
- after `1s`, will trigger `redelivery`
- receive 10 messages again

The real process:
- send 1 message
  - Reach the batch time limit, and submit a batch. return `msgId_1`
- send 9 messages in another batch
  - submit a batch. return `msgId_2`
- receive 10 messages, do negative acknowledge
  - push the `msgId_1` to `negativeAcksTracker`
  - push the `msgId_2` to `unAckedMessageTracker`
- after `1s`, will trigger redelivery `msgId_2` by `unAckedMessageTracker`
- receive 9 messages( `msgId_2` ) again
- after `60s`, will trigger redelivery `msgId_1` by `negativeAcksTracker`. <strong>(High light)</strong> Test execution timeout!
- receive 1 messages( `msgId_1` ) again

### Modifications

- remove conf: `ackTimeout`
- set `negativeAckRedeliveryDelay 1s`

### Documentation

- [x] `doc-not-needed`

### Matching PR in forked repository

PR in forked repository:

- poorbarcode#18

(cherry picked from commit 85b1138)
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.

Flaky-test: NegativeAcksTest.testNegativeAcksWithBatchAckEnabled
5 participants