[
https://issues.apache.org/jira/browse/HDDS-155?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16513041#comment-16513041
]
Hanisha Koneru edited comment on HDDS-155 at 6/14/18 9:59 PM:
--------------------------------------------------------------
Thanks for the update [~bharatviswa].
Some comments (Many of them are just NITs.):
# Can we rename the following in {{DatanodeContainerProtocol.proto}}
CONTAINER_REQUIRED_FILES_CREATE_ERROR -> CONTAINER_FILES_CREATE_ERROR
CONTAINER_CHECKSUM_FILE_CALCULATE_ERROR -> CONTAINER_CHECKSUM_ERROR
# {{KeyValueContainer}} Line 131, 133 : Instead of
"String.valueOf(containerId)", we can reuse containerName
# {{KeyValueContainer#updateRequiredFiles()}} -> when update succeeds, the
backup files must be deleted.
# In all scenarios where the createContainer fails, we should delete the
containerBase dir.
# Can we rename {{createRequiredFiles}} and {{updateRequiredFiles}} to
createContainerFile and updateContainerFile? Required doesn’t tell us instantly
what the files are. I think it is ok not to mention that the checksum file is
also created as part of createContainerFile.
# Line 225 : "Unable to delete container checksum backup file” -> Since we are
deleting the temporary file, we can have "Unable to delete container temporary
checksum file”.
# {{KeyValueContainer#updateRequiredFiles()}}, when throwing
INVALID_CONTAINER_STATE or when the restore fails, we should set the state of
the container to INVALID.
# When restoring from back up files, can we add a info log message saying
"update failed, so restoring the container files."
# Javadoc of computeChecksum is misleading. "Create checksum file of the
.container file.” -> “Computer checksum of the .container file”.
# Can we have a file handle to {{.db file}} in {{KeyValueContainerData}} as
this would be used by every key operation.
# {{KeyValueContainerUtil#verifyIsNewContainer()}} -> existence of
containerBasePath should be sufficient condition to fail createContainer.
was (Author: hanishakoneru):
Thanks for the update [~bharatviswa].
Some comments (Many of them are just NITs.):
# Can we rename the following in {{DatanodeContainerProtocol.proto}}
CONTAINER_REQUIRED_FILES_CREATE_ERROR -> CONTAINER_FILES_CREATE_ERROR
CONTAINER_CHECKSUM_FILE_CALCULATE_ERROR -> CONTAINER_CHECKSUM_ERROR
# {{KeyValueContainer}} Line 131, 133 : Instead of
"String.valueOf(containerId)", we can reuse containerName
# {{KeyValueContainer#updateRequiredFiles()}} -> when update succeeds, the
backup files must be deleted. In all scenarios where the createContainer fails,
we should delete the containerBase dir.
# Can we rename {{createRequiredFiles}} and {{updateRequiredFiles}} to
createContainerFile and updateContainerFile? Required doesn’t tell us instantly
what the files are. I think it is ok not to mention that the checksum file is
also created as part of createContainerFile.
# Line 225 : "Unable to delete container checksum backup file” -> Since we are
deleting the temporary file, we can have "Unable to delete container temporary
checksum file”.
# {{KeyValueContainer#updateRequiredFiles()}}, when throwing
INVALID_CONTAINER_STATE or when the restore fails, we should set the state of
the container to INVALID.
# When restoring from back up files, can we add a info log message saying
"update failed, so restoring the container files."
# Javadoc of computeChecksum is misleading. "Create checksum file of the
.container file.” -> “Computer checksum of the .container file”.
# Can we have a file handle to {{.db file}} in {{KeyValueContainerData}} as
this would be used by every key operation.
# {{KeyValueContainerUtil#verifyIsNewContainer()}} -> existence of
containerBasePath should be sufficient condition to fail createContainer.
> Implement KeyValueContainer and adopt new disk layout for the containers
> ------------------------------------------------------------------------
>
> Key: HDDS-155
> URL: https://issues.apache.org/jira/browse/HDDS-155
> Project: Hadoop Distributed Data Store
> Issue Type: Sub-task
> Reporter: Bharat Viswanadham
> Assignee: Bharat Viswanadham
> Priority: Major
> Fix For: 0.2.1
>
> Attachments: HDDS-155-HDDS-48.00.patch, HDDS-155-HDDS-48.01.patch,
> HDDS-155-HDDS-48.02.patch, HDDS-155-HDDS-48.03.patch,
> HDDS-155-HDDS-48.04.patch, HDDS-155-HDDS-48.05.patch
>
>
> This Jira is to add following:
> # Implement Container Interface
> # Use new directory layout proposed in the design document.
> a. Data location (chunks)
> b. Meta location (DB and .container files)
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
---------------------------------------------------------------------
To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org
