Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

change nondurable cursor to active #6769

Conversation

zhaorongsheng
Copy link
Contributor

Motivation

When use non-durable subscription the cursor is not active which lead to the written entries are not put into cache. This situation would degrade reading performance.

Modifications

Change the NonDurableCursorImpl to active and remove the three override methods: setActive()/isActive()/setInactive()

Verifying this change

This change added tests and can be verified as follows:

ManagedCursorTest.testNonDurableCursorActive()

  • add test to check NonDurableCursorImpl activity

Copy link
Contributor

@merlimat merlimat left a comment

Choose a reason for hiding this comment

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

This is not the right fix since it would cache even if the cursor is reading older data

@zhaorongsheng
Copy link
Contributor Author

This is not the right fix since it would cache even if the cursor is reading older data

@merlimat I think the cursor will be set inactive by the interval backlog check.

@merlimat
Copy link
Contributor

I think the cursor will be set inactive by the interval backlog check.

@zhaorongsheng You're probably right. Let me review this more in depth tomorrow since it was a long time since I touched it.

@zhaorongsheng
Copy link
Contributor Author

I think the cursor will be set inactive by the interval backlog check.

@zhaorongsheng You're probably right. Let me review this more in depth tomorrow since it was a long time since I touched it.

OK. Thanks~

@sijie sijie added this to the 2.6.0 milestone Apr 21, 2020
@sijie sijie added area/broker release/2.5.2 type/enhancement The enhancements for the existing features or docs. e.g. reduce memory usage of the delayed messages labels Apr 21, 2020
@sijie
Copy link
Member

sijie commented Apr 21, 2020

/pulsarbot run-failure-checks

@codelipenghui codelipenghui merged commit 16ab351 into apache:master Apr 22, 2020
addisonj pushed a commit to instructure/pulsar that referenced this pull request May 7, 2020
### Motivation

When use non-durable subscription the cursor is not active which lead to the written entries are not put into cache. This situation would degrade reading performance.

### Modifications

Change the `NonDurableCursorImpl` to active and remove the three override methods: `setActive()/isActive()/setInactive()`

### Verifying this change

This change added tests and can be verified as follows:

*ManagedCursorTest.testNonDurableCursorActive()*
  - add test to check `NonDurableCursorImpl` activity
jiazhai pushed a commit that referenced this pull request May 8, 2020
### Motivation

When use non-durable subscription the cursor is not active which lead to the written entries are not put into cache. This situation would degrade reading performance.

### Modifications

Change the `NonDurableCursorImpl` to active and remove the three override methods: `setActive()/isActive()/setInactive()`

### Verifying this change

This change added tests and can be verified as follows:

*ManagedCursorTest.testNonDurableCursorActive()*
  - add test to check `NonDurableCursorImpl` activity

(cherry picked from commit 16ab351)
jerrypeng pushed a commit to jerrypeng/incubator-pulsar that referenced this pull request May 15, 2020
### Motivation

When use non-durable subscription the cursor is not active which lead to the written entries are not put into cache. This situation would degrade reading performance.

### Modifications

Change the `NonDurableCursorImpl` to active and remove the three override methods: `setActive()/isActive()/setInactive()`

### Verifying this change

This change added tests and can be verified as follows:

*ManagedCursorTest.testNonDurableCursorActive()*
  - add test to check `NonDurableCursorImpl` activity
huangdx0726 pushed a commit to huangdx0726/pulsar that referenced this pull request Aug 24, 2020
### Motivation

When use non-durable subscription the cursor is not active which lead to the written entries are not put into cache. This situation would degrade reading performance.

### Modifications

Change the `NonDurableCursorImpl` to active and remove the three override methods: `setActive()/isActive()/setInactive()`

### Verifying this change

This change added tests and can be verified as follows:

*ManagedCursorTest.testNonDurableCursorActive()*
  - add test to check `NonDurableCursorImpl` activity
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/broker release/2.5.2 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.

None yet

4 participants