HDDS-7076. Log container file path when container cannot be written. #3684
Conversation
Fix error message
| @@ -260,7 +260,8 @@ private void writeToContainerFile(File containerFile, boolean isCreate) | |||
| } catch (IOException ex) { | |||
| onFailure(containerData.getVolume()); | |||
| throw new StorageContainerException("Error while creating/ updating " + | |||
| ".container file. ContainerID: " + containerId, ex, | |||
| ".container file. ContainerID: " + containerId + " Container path: " + | |||
There was a problem hiding this comment.
| ".container file. ContainerID: " + containerId + " Container path: " + | |
| " container file. ContainerID: " + containerId + ", container path: " + |
There was a problem hiding this comment.
I think the . was originally there to signify the file extension of the file is .container. I don't think it matters either way, just a note.
There was a problem hiding this comment.
Thanks for spotting this, I've fixed it.
|
Thanks for the patch @Galsza. LGTM, just one nitpicking comment inline. |
Galsza
force-pushed
the
HDDS-7076_log_container_file_path_when_container_can_not_be_written
branch
from
d8fe471
to
d22f9a4
Compare
| throw new StorageContainerException("Error while creating/ updating " + | ||
| ".container file. ContainerID: " + containerId, ex, | ||
| " container file. ContainerID: " + containerId + | ||
| " ,container path: " + containerFile.getAbsolutePath(), ex, |
There was a problem hiding this comment.
Note that error may happen for temp file (lines 246-248) or final file (lines 254, 256-257). Can we distinguish between those two cases?
Also, spacing is a bit odd here (some double, some missing). How about:
throw new StorageContainerException("Error while creating/updating" +
" container file. ContainerID: " + containerId +
", container path: " + containerFile.getAbsolutePath(), ex,There was a problem hiding this comment.
Added additional text when the temp file could not be created. I'm not sure what else we can do with this situation.
…emporary file was not created.
Galsza
force-pushed
the
HDDS-7076_log_container_file_path_when_container_can_not_be_written
branch
from
f42c2e4
to
960ca1b
Compare
|
@Galsza Please try to avoid force-push when updating the PR. Here are some great articles that explain why: https://developers.mattermost.com/blog/submitting-great-prs/#4-avoid-force-pushing |
|
Thanks @Galsza for updating the patch. Thanks @DuongNguyen0 for the review. |
What changes were proposed in this pull request?
Add container file path to error message when writing to container encounters an error
What is the link to the Apache JIRA
HDDS-7076
How was this patch tested?
Reproduce the error on a local cluster via taking away the write permission on container data by making its folder read only.