Skip to content

[Issue 7347] Avoid the NPE occurs in method ManagedLedgerImpl.isOffloadedNeedsDelete - #7389

Merged
sijie merged 7 commits into
apache:masterfrom
gaoran10:set-default-offload-deletion-lag
Jul 8, 2020
Merged

[Issue 7347] Avoid the NPE occurs in method ManagedLedgerImpl.isOffloadedNeedsDelete#7389
sijie merged 7 commits into
apache:masterfrom
gaoran10:set-default-offload-deletion-lag

Conversation

@gaoran10

@gaoran10 gaoran10 commented Jun 30, 2020 •
edited
Loading

Copy link
Copy Markdown
Contributor

Fixes #7347

Motivation

The default value of the offload-deletion-lag is null, this will cause an NPE problem.

Modifications

Add null check in the method ManagedLedgerImpl.isOffloadedNeedsDelete.

Verifying this change

Add unit test for method ManagedLedgerImpl.isOffloadedNeedsDelete.

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

Documentation

  • Does this pull request introduce a new feature? (no)

@gaoran10 gaoran10 changed the title [Offload] Set default value for offload-deletion-lag [Issue 7347] Set default value for offload-deletion-lag Jun 30, 2020
merlimat
merlimat reviewed Jun 30, 2020
View reviewed changes
Comment thread pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/OffloadPolicies.java Outdated
public final static String DEFAULT_OFFLOADER_DIRECTORY = "./offloaders";
public final static long DEFAULT_OFFLOAD_THRESHOLD_IN_BYTES = -1;
public final static Long DEFAULT_OFFLOAD_DELETION_LAG_IN_MILLIS = null;
public final static long DEFAULT_OFFLOAD_DELETION_LAG_IN_MILLIS = -1;

@merlimat merlimat Jun 30, 2020

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.

Should we also change managedLedgerOffloadDeletionLagInMillis to long?

@gaoran10 gaoran10 Jun 30, 2020

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This default is couldn't be Null, so I changed this field type to long.

@merlimat merlimat Jun 30, 2020

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.

Yes, it's fine. I was just saying to also change the type for the managedLedgerOffloadDeletionLagInMillis field which is currently Long. If null is not a valid value, we should use long type instead.

@gaoran10 gaoran10 Jul 1, 2020

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, I changed it. Thanks.

@codelipenghui codelipenghui added this to the 2.7.0 milestone Jul 1, 2020
@gaoran10

gaoran10 commented Jul 1, 2020

Copy link
Copy Markdown
Contributor Author

/pulsarbot run-failure-checks

1 similar comment
@codelipenghui

codelipenghui commented Jul 1, 2020

Copy link
Copy Markdown
Contributor

/pulsarbot run-failure-checks

@sijie

sijie commented Jul 2, 2020

Copy link
Copy Markdown
Member

/pulsarbot run-failure-checks

@sijie

sijie commented Jul 2, 2020

Copy link
Copy Markdown
Member

/pulsarbot run-failure-checks

@gaoran10

gaoran10 commented Jul 6, 2020

Copy link
Copy Markdown
Contributor Author

/pulsarbot run-failure-checks

@gaoran10 gaoran10 changed the title [Issue 7347] Set default value for offload-deletion-lag [Issue 7347] Avoid the NPE occurs in method ManagedLedgerImpl.isOffloadedNeedsDelete Jul 6, 2020
@gaoran10

gaoran10 commented Jul 6, 2020

Copy link
Copy Markdown
Contributor Author

/pulsarbot run-failure-checks

2 similar comments
@gaoran10

gaoran10 commented Jul 6, 2020

Copy link
Copy Markdown
Contributor Author

/pulsarbot run-failure-checks

@gaoran10

gaoran10 commented Jul 7, 2020

Copy link
Copy Markdown
Contributor Author

/pulsarbot run-failure-checks

@sijie
sijie merged commit eaf268c into apache:master Jul 8, 2020
codelipenghui pushed a commit to streamnative/pulsar-archived that referenced this pull request Jul 14, 2020
@gaoran10 @codelipenghui
[Issue 7347] Avoid the NPE occurs in method `ManagedLedgerImpl.isOffl…
575ce35 
…oadedNeedsDelete` (apache#7389)

Fixes apache#7347 

### Motivation

The default value of the offload-deletion-lag is `null`, this will cause an NPE problem.

### Modifications

Add null check in the method `ManagedLedgerImpl.isOffloadedNeedsDelete`.

### Verifying this change

Add unit test for method `ManagedLedgerImpl.isOffloadedNeedsDelete`.

(cherry picked from commit eaf268c)
@lukestephenson-zendesk

lukestephenson-zendesk commented Jul 15, 2020

Copy link
Copy Markdown
Contributor

I'm trying to work around this until 2.6.1 is released. Not sure what to do.

The default value of the offload-deletion-lag is null, this will cause an NPE problem

From the docs I thought the default value would be 14400000
image

Will explicitly setting a value for this setting help?

cdbartholomew pushed a commit to kafkaesque-io/pulsar that referenced this pull request Jul 21, 2020
@gaoran10 @cdbartholomew
[Issue 7347] Avoid the NPE occurs in method `ManagedLedgerImpl.isOffl…
e73383b 
…oadedNeedsDelete` (apache#7389)

Fixes apache#7347 

### Motivation

The default value of the offload-deletion-lag is `null`, this will cause an NPE problem.

### Modifications

Add null check in the method `ManagedLedgerImpl.isOffloadedNeedsDelete`.

### Verifying this change

Add unit test for method `ManagedLedgerImpl.isOffloadedNeedsDelete`.
cdbartholomew pushed a commit to kafkaesque-io/pulsar that referenced this pull request Jul 24, 2020
@gaoran10 @cdbartholomew
[Issue 7347] Avoid the NPE occurs in method `ManagedLedgerImpl.isOffl…
494e82b 
…oadedNeedsDelete` (apache#7389)

Fixes apache#7347 

### Motivation

The default value of the offload-deletion-lag is `null`, this will cause an NPE problem.

### Modifications

Add null check in the method `ManagedLedgerImpl.isOffloadedNeedsDelete`.

### Verifying this change

Add unit test for method `ManagedLedgerImpl.isOffloadedNeedsDelete`.
wolfstudy pushed a commit that referenced this pull request Jul 29, 2020
@gaoran10 @wolfstudy
[Issue 7347] Avoid the NPE occurs in method `ManagedLedgerImpl.isOffl…
0a2df5a 
…oadedNeedsDelete` (#7389)

Fixes #7347 

### Motivation

The default value of the offload-deletion-lag is `null`, this will cause an NPE problem.

### Modifications

Add null check in the method `ManagedLedgerImpl.isOffloadedNeedsDelete`.

### Verifying this change

Add unit test for method `ManagedLedgerImpl.isOffloadedNeedsDelete`.

(cherry picked from commit eaf268c)
huangdx0726 pushed a commit to huangdx0726/pulsar that referenced this pull request Aug 24, 2020
@gaoran10 @huangdx0726
[Issue 7347] Avoid the NPE occurs in method `ManagedLedgerImpl.isOffl…
237b5b9 
…oadedNeedsDelete` (apache#7389)

Fixes apache#7347 

### Motivation

The default value of the offload-deletion-lag is `null`, this will cause an NPE problem.

### Modifications

Add null check in the method `ManagedLedgerImpl.isOffloadedNeedsDelete`.

### Verifying this change

Add unit test for method `ManagedLedgerImpl.isOffloadedNeedsDelete`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@merlimat merlimat merlimat approved these changes
@sijie sijie sijie approved these changes
@codelipenghui codelipenghui codelipenghui approved these changes

Assignees

@gaoran10 gaoran10

Labels

release/2.6.1

Projects

None yet

Milestone

2.7.0

Development

Successfully merging this pull request may close these issues.

NPE occurs in the method ManagedLedgerImpl.isOffloadedNeedsDelete.

5 participants

@gaoran10 @codelipenghui @sijie @lukestephenson-zendesk @merlimat