kop icon indicating copy to clipboard operation
kop copied to clipboard

fix: ListOffset interface returns an incorrect result

Open keyboardbobo opened this issue 3 years ago • 2 comments

The commit offset and partition data expire at the same time, and the ListOffset interface returns an incorrect result

(If this PR fixes a github issue, please add Fixes #<xyz>.)

Fixes #1737

Motivation

The commit offset and partition data expire at the same time, and the ListOffset interface returns an incorrect result

Modifications

If all the data expires, the entry of lac becomes -1, and the actual offset will be 1 smaller than LATEST, add 1 back

Verifying this change

  • [x] Make sure that the change passes the CI checks.

(Please pick either of the following options)

This change is a trivial rework / code cleanup without any test coverage.

(or)

This change is already covered by existing tests, such as (please describe tests).

(or)

This change added tests and can be verified as follows:

(example:)

  • Added integration tests for end-to-end deployment with large payloads (10MB)
  • Extended integration test for recovery after broker failure

Documentation

Check the box below.

Need to update docs?

  • [ ] doc-required

    (If you need help on updating docs, create a doc issue)

  • [x] no-need-doc

    (Please explain why)

  • [ ] doc

    (If this PR contains doc changes)

keyboardbobo avatar Mar 01 '23 03:03 keyboardbobo

CLA assistant check
All committers have signed the CLA.

CLAassistant avatar Mar 01 '23 03:03 CLAassistant

This is an interesting finding. I am not sure that we can build a test.

How did you test that this fix actually fixed the problem ?

@eolivelli This is indeed difficult to test, but the problem does exist. I tested the following scenarios, and the producer and consumer offsets all need to meet expectations:

  1. __consumer_offset data has not expired, topic data has not expired
  2. __consumer_offset data is expired, topic data is not expired
  3. __consumer_offset data expires, topic data expires At present, only the consumer offset queried in the third case is wrong, The third case corresponds to if (position.compareTo(lac) > 0). I don't know if you have a better test method?

keyboardbobo avatar Mar 08 '23 02:03 keyboardbobo