[fix] [broker] Fix replicator send message error but cursor not set back to rewind position

[zhanghaou]

Motivation

When message send message failed and there is a pending read, cursor.rewind() set the read position back to mark delete position, but pending read will use old position, this will cause ack hole in replication cursor, and remote cluster will lost messages unless replicator reloaded.

We discovered this issue while developing rop, and I think it's a common problem with pulsar.

Here is some debug log, and I added some logs marked in circles.

1. add log when start readMoreEntries()

2. add log in send message error callback.

3. debug log when send message error.
   

Modifications

• Remove pending read when send message error.
• The failed scenario is difficult to reproduce, so I did not add a test cases.

Verifying this change

• 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

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
• The metrics
• Anything that affects deployment

Documentation

• doc
• doc-required
• doc-not-needed
• doc-complete

Matching PR in forked repository

PR in forked repository:

[hanmz]

Can we add a test for this case?

[hanmz]

I suspect this has something to do with the version you are using. I suggest you try it on the latest version.

[zhanghaou]

@hanmz Thanks for your reply. This problem also exists in the master branch. This problem not only occur in the case of geo-replication. This pip(#20469) plans to solve all similar problems, but there is no progress currently. The purpose of this PR is to fix the problem in the geo-replication scenario.

[hanmz]

Are there any concurrency issues here? Because the pending request is handled in another thread

[zhanghaou]

Indeed. Call cancelPendingReadRequest() first and then rewind cursor can reduce it.