[
https://issues.apache.org/jira/browse/HDFS-14994?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16978500#comment-16978500
]
Stephen O'Donnell commented on HDFS-14994:
------------------------------------------
Is there a bug in the current implementation I cannot see, or is this just a
desire to refactor this a little?
In the original code, and as per the 001 patch, you cannot just swap continue
for break as it stops the priority variable getting incremented on the last
pass, and then the later if statement will fail:
{code}
if (priority == LEVEL) {
// Reset all bookmarks because there were no recently added blocks.
for (LightWeightLinkedSet<BlockInfo> q : priorityQueues) {
q.resetBookmark();
}
}
{code}
The 003 patch looks functionally correct to me, however we have only really
saved an IF statement to remove the continue and gained an extra condition in
the later IF statement, so I am not really sure if this is better or worse than
before (assuming the original does not have a bug I cannot spot right now):
{code}
if (count < blocksToProcess && priority == QUEUE_WITH_CORRUPT_BLOCKS) {
// Reset all bookmarks because there were no recently added blocks.
for (LightWeightLinkedSet<BlockInfo> q : priorityQueues) {
q.resetBookmark();
}
}
{code}
> Optimize LowRedundancyBlocks#chooseLowRedundancyBlocks()
> --------------------------------------------------------
>
> Key: HDFS-14994
> URL: https://issues.apache.org/jira/browse/HDFS-14994
> Project: Hadoop HDFS
> Issue Type: Improvement
> Reporter: Lisheng Sun
> Assignee: Lisheng Sun
> Priority: Major
> Attachments: HDFS-14994.001.patch, HDFS-14994.002.patch,
> HDFS-14994.003.patch
>
>
> when priority=QUEUE_WITH_CORRUPT_BLOCKS, it mean no block in needreplication
> need replica.
> in current code if use continue, there is one more invalid judgment (priority
> ==QUEUE_WITH_CORRUPT_BLOCKS).
> i think it should use break instread of continue.
> {code:java}
> */
> synchronized List<List<BlockInfo>> chooseLowRedundancyBlocks(
> int blocksToProcess) {
> final List<List<BlockInfo>> blocksToReconstruct = new ArrayList<>(LEVEL);
> int count = 0;
> int priority = 0;
> for (; count < blocksToProcess && priority < LEVEL; priority++) {
> if (priority == QUEUE_WITH_CORRUPT_BLOCKS) {
> // do not choose corrupted blocks.
> continue;
> }
> ...
>
> }
> {code}
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
