Skip to content

Conversation

@merlimat
Copy link
Contributor

Motivation

Currently there's a rate limiter that avoid evicting from cache each time the read position is updated but rather only does that every 1 sec at max. The primary reason is to avoid mutex contention across different cursors trying to trigger the eviction in parallel.

The frequency should be higher though, to make sure we promptly discard all entries once all active cursors have moved past a particular point.

Modifications

  • Allow to configure the frequency from broker.conf
  • Increased default to 100/s

@merlimat merlimat added the type/enhancement The enhancements for the existing features or docs. e.g. reduce memory usage of the delayed messages label Apr 17, 2019
@merlimat merlimat added this to the 2.4.0 milestone Apr 17, 2019
@merlimat merlimat self-assigned this Apr 17, 2019
@merlimat merlimat force-pushed the conf-cache-eviction-freq branch from 4f12504 to 7facb33 Compare April 17, 2019 18:56
@merlimat merlimat force-pushed the conf-cache-eviction-freq branch from 22eaac4 to 60cda05 Compare April 24, 2019 00:25
Copy link
Contributor

@rdhabalia rdhabalia left a comment

Choose a reason for hiding this comment

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

minor comment regarding test.

// activate caught up cursors
cursors.forEach(cursor -> {
if (cursor.getNumberOfEntries() < maxActiveCursorBacklogEntries) {
if (cursor.getNumberOfEntries() < backloggedCursorThresholdEntries) {
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@merlimat
Copy link
Contributor Author

@rdhabalia Updated with an additional time threshold and check for slowest active consumer. Please take another look.

Copy link
Contributor

@rdhabalia rdhabalia left a comment

Choose a reason for hiding this comment

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

LGTM.. few minor comments..

}
// this task: helps to activate inactive-backlog-cursors which have caught up and
// connected, also deactivate active-backlog-cursors which has backlog
((PersistentTopic) topic).getManagedLedger().checkBackloggedCursors();
Copy link
Contributor

Choose a reason for hiding this comment

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

if we have removed from here then who triggers checkBackloggedCursors(..)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

uhm.. :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

Copy link
Contributor

@rdhabalia rdhabalia left a comment

Choose a reason for hiding this comment

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

👍

@rdhabalia
Copy link
Contributor

rdhabalia commented Apr 24, 2019

@merlimat we may also want to add config to standalone-config file and site-documentation file.

@merlimat
Copy link
Contributor Author

@merlimat we may also want to add config to config files.

Which one are you referring? I added into broker.conf

@rdhabalia
Copy link
Contributor

Which one are you referring? I added into broker.conf

may be we can add configs to :
https://github.com/apache/pulsar/blob/master/site2/docs/reference-configuration.md
https://github.com/apache/pulsar/blob/master/conf/standalone.conf

@merlimat
Copy link
Contributor Author

Good point, Added.

Copy link
Contributor

@ivankelly ivankelly left a comment

Choose a reason for hiding this comment

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

Is there any testing for the time based eviction? I don't see it

@merlimat
Copy link
Contributor Author

@ivankelly Good point. Added explicit test for time-based eviction.

@merlimat
Copy link
Contributor Author

merlimat commented May 2, 2019

run java8 tests

@merlimat merlimat merged commit f5c7b22 into apache:master May 2, 2019
sijie pushed a commit that referenced this pull request Jan 18, 2020
### Motivation
Some parameters are added in the `broker.conf` and `standalone.conf` files. However, those parameters are not updated in the docs.
See the following PRs for details: #4150, #4066, #4197, #3819, #4261, #4273, #4320.

### Modifications
Add those parameter info, and sync docs with the code.

Does not update the description quite much, there are two reasons for this:
1. Keep doc content consistent with code. We need to update the description for those parameters in the code first, and then sync them in docs.
2. Will adopt a generator to generate those content automatically in the near future.
huangdx0726 pushed a commit to huangdx0726/pulsar that referenced this pull request Aug 24, 2020
### Motivation
Some parameters are added in the `broker.conf` and `standalone.conf` files. However, those parameters are not updated in the docs.
See the following PRs for details: apache#4150, apache#4066, apache#4197, apache#3819, apache#4261, apache#4273, apache#4320.

### Modifications
Add those parameter info, and sync docs with the code.

Does not update the description quite much, there are two reasons for this:
1. Keep doc content consistent with code. We need to update the description for those parameters in the code first, and then sync them in docs.
2. Will adopt a generator to generate those content automatically in the near future.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type/enhancement The enhancements for the existing features or docs. e.g. reduce memory usage of the delayed messages

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants