Skip to content

[fix] Exchange data cleanup improvement#698

Open
gaoran10 wants to merge 5 commits intostreamnative:masterfrom
gaoran10:exchange-data-clear
Open

[fix] Exchange data cleanup improvement#698
gaoran10 wants to merge 5 commits intostreamnative:masterfrom
gaoran10:exchange-data-clear

Conversation

@gaoran10
Copy link
Copy Markdown
Collaborator

@gaoran10 gaoran10 commented Oct 10, 2022

Motivation

The PR #619 changed the queue cursor to be durable, this will cause the exchange topic can't be cleaned up completely because one queue only handles part of the exchange data, and only these index positions could be acknowledged.

image

At checkpoint1, get the route mark delete position of exchange1 and exchange2, then get the last confirm position of the queue. The exchange1 route mark delete pos is 1:2, the exchange2 mark delete pos is 2:2, and the LAC of the queue may be equal with 3:2 or greater than 3:2 (because the routing message process is still running), when the mark delete pos of the queue reach the LAC at checkpoint1, it indicates that the index message 3:0, 3:1 and 3:2 were all acked, so we can ack the exchange1 with pos 1:2, and ack the exchange2 with pos 2:2.

Modifications

Add exchange data cleanup scheduled task for each queue.
Remove useless unAckMessages in AmqpConsumer.

Verifying this change

Add verification to make sure exchange data could be cleaned up completely.

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: (no)
  • The rest endpoints: (no)
  • The admin cli options: (no)
  • Anything that affects deployment: (no)

Documentation

Check the box below.

Need to update docs?

  • doc-required

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

  • no-need-doc

    (Please explain why)

  • doc

    (If this PR contains doc changes)

@gaoran10 gaoran10 requested review from a team and codelipenghui as code owners October 10, 2022 08:18
@github-actions github-actions Bot added the no-need-doc This pr does not need any document label Oct 10, 2022
queueName, position.getLedgerId(), position.getEntryId()));
}
}
FutureUtil.waitForAll(futures).thenRun(this::scheduleExchangeClearTask);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When (router.getExchange().markDeleteAsync) throws an exception, the task will stop. Replace (thenRun) with (whenComplete)

Copy link
Copy Markdown
Collaborator Author

@gaoran10 gaoran10 Oct 14, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch!

Fixed, please take a look again, thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-need-doc This pr does not need any document

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants