[jira] [Comment Edited] (HDDS-226) Client should update block length in OM while committing the key
[ https://issues.apache.org/jira/browse/HDDS-226?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16562502#comment-16562502 ] Tsz Wo Nicholas Sze edited comment on HDDS-226 at 7/30/18 11:29 PM: Thanks for the update. Just some minor comments: - addEntryToLocationInfoList(..) and getKeylength() are only used once. Let's remove them. - The changes in OmKeyLocationInfoGroup are not used. Please revert them. +1 patch looks good otherwise. was (Author: szetszwo): Thanks for the update. Just some minor comments: - addEntryToLocationInfoList(..) and getKeylength() are only used once. Let's remove them. - The changes in OmKeyLocationInfoGroup are not used. Please revert them. > Client should update block length in OM while committing the key > > > Key: HDDS-226 > URL: https://issues.apache.org/jira/browse/HDDS-226 > Project: Hadoop Distributed Data Store > Issue Type: Bug > Components: Ozone Manager >Reporter: Mukul Kumar Singh >Assignee: Shashikant Banerjee >Priority: Major > Fix For: 0.2.1 > > Attachments: HDDS-226.00.patch, HDDS-226.01.patch, HDDS-226.02.patch, > HDDS-226.03.patch, HDDS-226.04.patch, HDDS-226.05.patch, HDDS-226.06.patch, > HDDS-226.07.patch > > > Currently the client allocate a key of size with SCM block size, however a > client can always write smaller amount of data and close the key. The block > length in this case should be updated on OM. -- 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
[jira] [Comment Edited] (HDDS-226) Client should update block length in OM while committing the key
[ https://issues.apache.org/jira/browse/HDDS-226?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16559179#comment-16559179 ] Mukul Kumar Singh edited comment on HDDS-226 at 7/27/18 3:01 AM: - Thanks for working on this [~shashikant]. Please find me comments as following 1) OmKeyInfo#updateBlockLength, there are 2 for loops. Normally, the order of the blocks in the ksmKeyLocations and in the blockIDList will be the same, so I feel this can be optimized by walking the list only once. Also once we have found a match, we should break from the first loop. 2) Also we have a DatanodeBlockID in DatanodeContainerProto, the block length is not an argument there. Should this be updated as well ? was (Author: msingh): Thanks for working on this [~shashikant]. Apart from Ni 1) OmKeyInfo#updateBlockLength, there are 2 for loops. Normally, the order of the blocks in the ksmKeyLocations and in the blockIDList will be the same, so I feel this can be optimized by walking the list only once. Also once we have found a match, we should break from the first loop. 2) Also we have a DatanodeBlockID in DatanodeContainerProto, the block length is not an argument there. Should this be updated as well ? > Client should update block length in OM while committing the key > > > Key: HDDS-226 > URL: https://issues.apache.org/jira/browse/HDDS-226 > Project: Hadoop Distributed Data Store > Issue Type: Bug > Components: Ozone Manager >Reporter: Mukul Kumar Singh >Assignee: Shashikant Banerjee >Priority: Major > Fix For: 0.2.1 > > Attachments: HDDS-226.00.patch, HDDS-226.01.patch, HDDS-226.02.patch, > HDDS-226.03.patch, HDDS-226.04.patch, HDDS-226.05.patch > > > Currently the client allocate a key of size with SCM block size, however a > client can always write smaller amount of data and close the key. The block > length in this case should be updated on OM. -- 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
[jira] [Comment Edited] (HDDS-226) Client should update block length in OM while committing the key
[
https://issues.apache.org/jira/browse/HDDS-226?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16541502#comment-16541502
]
Shashikant Banerjee edited comment on HDDS-226 at 7/12/18 11:42 AM:
Thanks [~anu], for the review comments.
{quote}Another orthogonal Question: Sorry for these random comments. I see we
have BlockID class then we create a new class called OmBlockInfo and add one
more field, blockLength. Why is this not added as part of BlockID
{quote}
As a part of BlockCommitProtocol, we need to update each Block with the actual
block length written as well as BlockCommitSequenceId.
BlockId class as i suppose is used for identifying a unique block across the
system. So, I thought it would be less confusing to maintain per Block
associated Info for OzoneManager in a separate class. BlockCommitSequenceId
will be added to the OzoneBlockInfo class in subsequent patches(Mentioned in a
TODO item)
{quote}I am very confused with this code. Can you please check? Why are
checking keyArgs, did you intend to check blockInfoList?
{quote}
Yes, it was a mistake. Addressed in the latest patch.
{quote}OmKeysArgs.java: More of a question, when would this sum be not equal to
the datasize ?
{quote}
In the earlier patch, when we create a key we set Keylength and once we commit
the key to OM, it was just updating each of the blockLengths , not exactly
updating total size. Hence, once the key is committed, the actual key size will
be determined by the sum total of all block lengths.
Patch v2 now, updates each of the block lengths as well as the total size of
the Key, so the related code which you pointed out is removed.
Rest all the review comments are addressed.
was (Author: shashikant):
Thanks [~anu], for the review comments.
{quote}
Another orthogonal Question: Sorry for these random comments. I see we have
BlockID class then we create a new class called OmBlockInfo and add one more
field, blockLength. Why is this not added as part of BlockID
{quote}
As a part of BlockCommitProtocol, we need to update each Block with the actual
block length written as well as BlockCommitSequenceId.
BlockId class as i suppose is used for identifying a unique block across the
system. So, I thought it would be less confusing to maintain
per Block associated Info for OzoneManager in a separate class.
BlockCommitSequenceId will be added to the OzoneBlockInfo class in subsequent
patches(Mentioned in a TODO item)
{quote}
I am very confused with this code. Can you please check? Why are checking
keyArgs, did you intend to check blockInfoList?
{quote}
Yes, it was a mistake. Addressed in the latest patch.
{quote}
OmKeysArgs.java: More of a question, when would this sum be not equal to the
datasize ?
{quote}
In the earlier patch, when we create a key we set Keylength and once we commit
the key to OM, it was just updating each of the blockLengths , not exactly
updating total size. Hence, once the key is committed, the actual key size will
be determined by the sum total of all block lengths.
Patch v2 now, updates each of the block lengths as well as the total size of
the Key, so the related code which you pointed out is removed.
Rest all the review comments are addressed.
> Client should update block length in OM while committing the key
>
>
> Key: HDDS-226
> URL: https://issues.apache.org/jira/browse/HDDS-226
> Project: Hadoop Distributed Data Store
> Issue Type: Bug
> Components: Ozone Manager
>Reporter: Mukul Kumar Singh
>Assignee: Shashikant Banerjee
>Priority: Major
> Fix For: 0.2.1
>
> Attachments: HDDS-226.00.patch, HDDS-226.01.patch, HDDS-226.02.patch
>
>
> Currently the client allocate a key of size with SCM block size, however a
> client can always write smaller amount of data and close the key. The block
> length in this case should be updated on OM.
--
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
