[improve][pip] PIP-437: Granular and Fixed-Delay Policies for Message Delivery

[chrisqwang3]

PIP: 437

Motivation

The current system lacks administrative controls on message delivery delays, which introduces risks to cluster stability and data integrity. This proposal aims to provide granular control at the topic and namespace levels to add a fixed delay delivery configuration

Modifications

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:

[github-actions]

@chrisqwang3 Please add the following content to your PR description and select a checkbox:

- [ ] `doc` <!-- Your PR contains doc changes -->
- [ ] `doc-required` <!-- Your PR changes impact docs and you will update later -->
- [ ] `doc-not-needed` <!-- Your PR changes do not impact docs -->
- [ ] `doc-complete` <!-- Docs have been already added -->

[lhotari]

Thanks for the proposal! Please check the review comments.

[lhotari]

@chrisqwang3 Your email message to the Pulsar dev mailing list had gone into the moderation queue. I released it from the queue today: https://lists.apache.org/thread/23t4zzbfjrm5r4pbzrofn3d85c0171yn . Please make sure to subscribe to the dev mailing list unless you have already done that. Instructions.

[lhotari]

LGTM. Great work @chrisqwang3

[lhotari]

@chrisqwang3 Thank you for driving this proposal.
Please go ahead and start the PIP vote thread since this PIP has been in discussion for some time.
Here's an example of how I've started vote threads: https://lists.apache.org/thread/ndh0dhfjqf1htcmodgyyqt6k4bgdfj3x

[Denovo1998]

@lhotari @coderzc @chrisqwang3
Why do we already have max-delivery-delay, and we still need a fixed-delivery-delay? I understand that this PIP is to fix the delay time of the message, but it feels strange when I see that fixed-delivery-delay can overwrite max-delivery-delay.

Why don‘t we continue to use max-delivery-delay, and then add a configuration similar to delivery-delay-type? It can be FIXED or VARIABLE. If it is FIXED, it is a fixed delay time.

[lhotari]

@lhotari @coderzc @chrisqwang3 Why do we already have max-delivery-delay, and we still need a fixed-delivery-delay? I understand that this PIP is to fix the delay time of the message, but it feels strange when I see that fixed-delivery-delay can overwrite max-delivery-delay.

@Denovo1998 There is a reason. When all messages in a topic have the same delay, there will be significantly less state to manage. A system can be designed in a way where topics are used as schedulers and this will also work at very high scale without issues. This would work well with InMemoryDelayedDeliveryTracker since it has the delayedDeliveryFixedDelayDetectionLookahead feature (#16609, #17907). There's no need to build a large index of the delayed messages.

pulsar/conf/broker.conf

Lines 659 to 663 in 84205eb

	# Size of the lookahead window to use when detecting if all the messages in the topic
	# have a fixed delay for InMemoryDelayedDeliveryTracker (the default DelayedDeliverTracker).
	# Default is 50,000. Setting the lookahead window to 0 will disable the logic to handle
	# fixed delays in messages in a different way.
	delayedDeliveryFixedDelayDetectionLookahead=50000

btw. It would be useful if there would be a way to specify delayedDeliveryTrackerFactoryClassName in a namespace level or topic level policy since the fixed delivery delay feature is mainly useful with InMemoryDelayedDeliveryTracker and the fixed delay detection. At the same time, there might be a requirement to use BucketDelayedDeliveryTrackerFactory for other delay delivery use cases.

Why don‘t we continue to use max-delivery-delay, and then add a configuration similar to delivery-delay-type? It can be FIXED or VARIABLE. If it is FIXED, it is a fixed delay time.

Here's the current DelayedDeliveryPolicies class:
https://github.com/apache/pulsar/blob/master/pulsar-client-admin-api/src/main/java/org/apache/pulsar/common/policies/data/DelayedDeliveryPolicies.java#L23-L41

I guess the reason is that we would break backwards compatibility if we'd make changes in the way that you are suggesting.

[Denovo1998]

Oh, it‘s very clear. I see. +1

[lhotari]

LGTM @chrisqwang3. I'll close the vote thread as accepted and merge this PIP PR.