Skip to content

Conversation

@codelipenghui
Copy link
Contributor

@codelipenghui codelipenghui commented Nov 13, 2019

Motivation

Since #5491 merged, while user use new pulsar client to produce batch messages to older version broker(e.g. 2.4.0), send ack error will occur:

[pulsar-client-io-8-2] WARN  org.apache.pulsar.client.impl.ProducerImpl - [persistent://sandbox/pressure-test/test-A-partition-11] [pulsar-cluster-test-13-294] Got ack for msg. expecting: 13 - got: 224 - queue-size: 9

The problem is client use highest sequence id to match the response sequence id, but in old version broker can not return the highest id.

So, this pr is try to fix the problem of produce batch message with new version client and old version broker.

Modifications

Add highest sequence id to CommandSendReceipt. If the response highest sequence id of send receipt > lowest sequence id, it means broker is a new version broker, so we need to verify the highest sequence id, otherwise we only verify the lowest sequence id.

Verifying this change

I have test on my mac book by using pulsar-perf,

new version client sends messages to old version broker(2.4.0) passed.
old version client sends messages to new version broker passed.

Backward compatibility test is added.

Does this pull request potentially affect one of the following parts:

If yes was chosen, please highlight the changes

  • Dependencies (does it add or upgrade a dependency): (no)
  • The public API: (no)
  • The schema: (no)
  • The default values of configurations: (no)
  • The wire protocol: (yes)
  • The rest endpoints: (no)
  • The admin cli options: (no)
  • Anything that affects deployment: (no)

Documentation

  • Does this pull request introduce a new feature? (no)

@codelipenghui
Copy link
Contributor Author

integration tests will added

@jiazhai
Copy link
Member

jiazhai commented Nov 14, 2019

run cpp tests
run java8 tests

@codelipenghui
Copy link
Contributor Author

backward compatibility(for 2.2.0, 2.3.0, 2.40) check is added, please help review this PR.

@codelipenghui
Copy link
Contributor Author

run java8 tests

2 similar comments
@codelipenghui
Copy link
Contributor Author

run java8 tests

@codelipenghui
Copy link
Contributor Author

run java8 tests

@sijie sijie modified the milestones: 2.4.3, 2.5.0 Nov 15, 2019
@sijie sijie merged commit 8a7d690 into apache:master Nov 15, 2019
@codelipenghui codelipenghui deleted the fix_send_ack_sequence branch May 19, 2021 05:36
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.

3 participants