[
https://issues.apache.org/jira/browse/HDFS-9809?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15371440#comment-15371440
]
Virajith Jalaparti edited comment on HDFS-9809 at 7/11/16 7:35 PM:
-------------------------------------------------------------------
Hi [~eddyxu],
Thank you for the comments! Replies below.
bq. {{BlockSender#waitForMinLength}}, it is not clear to me that why you need
to change {{RBW}} to {{RIP}} ?
This should have actually been {{ReplicaInPipelineInterface}} (or rather a
class that extends {{ReplicaInfo}} and implements
{{ReplicaInPipelineInterface}}) instead of {{ReplicaInPipeline}}. The idea in
replacing this was: when we add a {{ProvidedReplica}} as part of HDFS-9806, the
{{ProvidedReplica}} would be implementing {{ReplicaInPipelineInterface}} so
that it can tie into the existing replication pipeline in the datanode. So, if
references to {{ReplicaInPipeline}} and {{ReplicaBeingWritten}} are replaced by
{{ReplicaInPipelineInterface}}, parts of the current code can be used for both
{{LocalReplica}} and {{ProvidedReplica}}. The current patch does not have these
replacements as implementing writes for {{ProvidedReplica}} was future work of
HDFS-9806 and not in its scope. I understand that this should not preclude
replacement by {{ReplicaInPipelineInterface}} as it would eventually be needed.
I will make these modifications and post a new patch soon.
bq. {{DataStorage#getTrashDirectoryForBlockFile}} we can take this chance to
rename it to {{getTrashDirectoryForBlock}} or {{..ForReplica}}.
Agreed. Will do.
bq. There are many {{UnsupportedOperationException}} in Replica class
hierarchy. It might indicates that these functions are not supposed to be
override.
As some of the functions were moved up from the Replica class hierarchy (e.g.,
RIP etc. to {{ReplicaInfo}}) and other sub-classes of {{ReplicaInfo}} may not
implement it, these were declared to throw {{UnsupportedOperationException}}. I
agree that this is not necessary as it is a {{RuntimeException}}. I will update
the patch without the {{UnsupportedOperationException}}.
bq. ASF License for every new file.
Will add it.
bq. Lets take this chance to use JDK 7 {{try..resource}} in
{{breakHardlilnks()}} ?
Sure. Will change to use {{try..resource}}.
bq. {{ReplicaInPipeline#moveReplicaFrom}} it still has a {{File}} parameter.
Yes, this needs to be removed. This will be part of the changes to be made
following the first point above.
was (Author: virajith):
Hi [~eddyxu],
Thank you for the comments! Replies below.
bq. {{BlockSender#waitForMinLength}}, it is not clear to me that why you need
to change {{RBW}} to {{RIP}} ?
This should have actually been {{ReplicaInPipelineInterface}} instead of
{{ReplicaInPipeline}}. The idea in replacing this was: when we add a
{{ProvidedReplica}} as part of HDFS-9806, the {{ProvidedReplica}} would be
implementing {{ReplicaInPipelineInterface}} so that it can tie into the
existing replication pipeline in the datanode. So, if references to
{{ReplicaInPipeline}} and {{ReplicaBeingWritten}} are replaced by
{{ReplicaInPipelineInterface}}, parts of the current code can be used for both
{{LocalReplica}} and {{ProvidedReplica}}. The current patch does not have these
replacements as implementing writes for {{ProvidedReplica}} was future work of
HDFS-9806 and not in its scope. I understand that this should not preclude
replacement by {{ReplicaInPipelineInterface}} as it would eventually be needed.
I will make these modifications and post a new patch soon.
bq. {{DataStorage#getTrashDirectoryForBlockFile}} we can take this chance to
rename it to {{getTrashDirectoryForBlock}} or {{..ForReplica}}.
Agreed. Will do.
bq. There are many {{UnsupportedOperationException}} in Replica class
hierarchy. It might indicates that these functions are not supposed to be
override.
As some of the functions were moved up from the Replica class hierarchy (e.g.,
RIP etc. to {{ReplicaInfo}}) and other sub-classes of {{ReplicaInfo}} may not
implement it, these were declared to throw {{UnsupportedOperationException}}. I
agree that this is not necessary as it is a {{RuntimeException}}. I will update
the patch without the {{UnsupportedOperationException}}.
bq. ASF License for every new file.
Will add it.
bq. Lets take this chance to use JDK 7 {{try..resource}} in
{{breakHardlilnks()}} ?
Sure. Will change to use {{try..resource}}.
bq. {{ReplicaInPipeline#moveReplicaFrom}} it still has a {{File}} parameter.
Yes, this needs to be removed. This will be part of the changes to be made
following the first point above.
> Abstract implementation-specific details from the datanode
> ----------------------------------------------------------
>
> Key: HDFS-9809
> URL: https://issues.apache.org/jira/browse/HDFS-9809
> Project: Hadoop HDFS
> Issue Type: Task
> Components: datanode, fs
> Reporter: Virajith Jalaparti
> Assignee: Virajith Jalaparti
> Attachments: HDFS-9809.001.patch, HDFS-9809.002.patch,
> HDFS-9809.003.patch
>
>
> Multiple parts of the Datanode (FsVolumeSpi, ReplicaInfo, FSVolumeImpl etc.)
> implicitly assume that blocks are stored in java.io.File(s) and that volumes
> are divided into directories. We propose to abstract these details, which
> would help in supporting other storages.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
