Skip to content

Conversation

@sumitagrawl
Copy link
Contributor

@sumitagrawl sumitagrawl commented Jan 4, 2024

What changes were proposed in this pull request?

when container db is not available, throw IOException so that its not loaded to memory. Log also represent for not loading the container but seems not handled properly.

What is the link to the Apache JIRA

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

How was this patch tested?

  • Unit testcase added

Copy link
Contributor

@errose28 errose28 left a comment

Choose a reason for hiding this comment

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

Thanks for fixing this @sumitagrawl. Change LGTM but can you add a test as well to make sure that invalid containers missing a container file or DB are not loaded into the datanode on startup?

"Skipping loading of this container.", containerID);
// Don't further process this container, as it is missing db file.
return;
throw new IOException("Container DB file is missing");
Copy link
Contributor

Choose a reason for hiding this comment

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

Include the container ID in the exception.

@sumitagrawl sumitagrawl requested a review from errose28 January 5, 2024 07:37
@sumitagrawl sumitagrawl requested a review from kerneltime January 5, 2024 07:39
Copy link
Contributor

@errose28 errose28 left a comment

Choose a reason for hiding this comment

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

Thanks for the updates @sumitagrawl LGTM just one minor comment.

Co-authored-by: Ethan Rose <[email protected]>
Copy link
Contributor

@errose28 errose28 left a comment

Choose a reason for hiding this comment

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

Thanks @adoroszlai LGTM once CI passes.

@adoroszlai adoroszlai merged commit 702a385 into apache:master Jan 6, 2024
@adoroszlai
Copy link
Contributor

Thanks @sumitagrawl for the patch, @errose28, @kerneltime for the review.

ivandika3 pushed a commit to ivandika3/ozone that referenced this pull request Oct 12, 2025
swamirishi pushed a commit to swamirishi/ozone that referenced this pull request Dec 3, 2025
…tainer DB (apache#5921)

Change-Id: If44641fac4b202e8ada1bf1012f3e4087d8fa7cd
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.

4 participants