Skip to content

HDDS-12040. ozone freon cr fails with NPE in ReplicationSupervisor#7776

Merged
adoroszlai merged 10 commits intoapache:masterfrom
len548:HDDS-12040
Jan 30, 2025
Merged

HDDS-12040. ozone freon cr fails with NPE in ReplicationSupervisor#7776
adoroszlai merged 10 commits intoapache:masterfrom
len548:HDDS-12040

Conversation

@len548
Copy link
Contributor

@len548 len548 commented Jan 29, 2025

What changes were proposed in this pull request?

"ozone freon cr" command runs into exceptions because requestCounter map in ReplicationSupervisor class is not initiated during the following reproducing steps. addTask method in the class is supposed to initiate it but it is not called, so I extracted that part into a new method called initCounters and call this new method before running the task.

How to reproduce the error:

// create a key
ozone freon ockg -n 10
// close a container
ozone admin container close 1
// run the command
ozone freon cr -n 1

Also this PR handles the followings to make it work:

  • ClosedContainerReplicator needs update for HDDS-10804
  • It needs DB volumes with directories (HDDS-10230).

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-12040

How was this patch tested?

Manually tested

@adoroszlai
Copy link
Contributor

Thanks @len548 for the patch. Please update your branch by merging master into it and resolving the conflict.

@adoroszlai adoroszlai changed the title HDDS-12040. "ozone freon cr" does not work HDDS-12040. ozone freon cr fails with NPE in ReplicationSupervisor Jan 29, 2025
Copy link
Contributor

@adoroszlai adoroszlai left a comment

Choose a reason for hiding this comment

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

Thanks @len548 for updating the patch.

@len548
Copy link
Contributor Author

len548 commented Jan 30, 2025

@adoroszlai, thank you for the review and correction!

Copy link
Contributor

@adoroszlai adoroszlai left a comment

Choose a reason for hiding this comment

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

Thanks @len548 for iterating on this.

Sorry for nitpicking about method order and whitespace. If you look at the changes in ReplicationSupervisor you may see why it was important: makes review much easier. Now it's clear that addTask was just split into 3 methods, instead of having to inspect blocks of code line-by-line.

@len548
Copy link
Contributor Author

len548 commented Jan 30, 2025

@adoroszlai thank you for teaching this practice throughly and sorry for causing unnecessary changes again and again. I often miss these, especially the unnecessary whitespaces because the changes are not visually obvious on the IntelliJ ;(

@adoroszlai adoroszlai merged commit 4fc4a9a into apache:master Jan 30, 2025
42 checks passed
nandakumar131 pushed a commit to nandakumar131/ozone that referenced this pull request Feb 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants