[
https://issues.apache.org/jira/browse/HDFS-12283?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16135456#comment-16135456
]
Weiwei Yang commented on HDFS-12283:
------------------------------------
Hi [~yuanbo]
A few comments to v6 patch
*ozone-default.xml*
The property value of "ozone.scm.block.deletion.max.retry" should be 4096 (same
as java constant) instead of 5 here.
*DeletedBlockLog.java*
NIT: line 33: Returns a certain size list of transactions -> a limit size list
of transactions. Note count is the max number of TXs to return, we might not be
able to always return this number.
*DeletedBlockLogImpl.java*
# NIT: can we move line 67 - 71 to {{getTransactions}} method since they were
only used there, we don't need them globally.
# line 64: can we use AtomicLong for {{latestTxid}}?
# line 83: add {{setCreateIfMissing(true)}}?
# line 158 : please add code to handle potential null value of {{block}} in
case the given txid not exist in the log
# line 169: please remove this warning message, it's not appropriate to print
this warning from the log
# line 172: I noticed you used a {{BatchOperation}} to atomic update all TXs,
but is that really necessary? If one we are not able to update one TX here, it
should not stop us updating rest of TXs in the list, so I suggest to remove the
batch operation here. If a TX failed to update, print a warning and continue
with next.
# line 194: I don't think we should catch this exception and just print a
warning, better to throw this out and let the caller to decide how to proceed.
# exception handling of {{addTransaction}}, what if
{{deletedStore.writeBatch(batch)}} fails and throws an IOException, in that
case latestTxid is not updated to the log but updated in memory?
# One more question, would {{getTransactions}} iterate over the log over again
if there is no enough TX to return? For example if the log has TXs like
\[1,2,3,4,5\], first getTransactions(4) returns \[1,2,3,4\], then I call
getTransactions(4) will it return \[5,1,2,3\] ? Another problem, it looks like
in line 122 you fetch {{count}} number of TXs and use that for the base, but
what if it contains some -1 and get skipped some TXs? Will I get a result less
than count?
I haven't looked into the test case yet, will do tomorrow. Thanks.
> Ozone: DeleteKey-5: Implement SCM DeletedBlockLog
> -------------------------------------------------
>
> Key: HDFS-12283
> URL: https://issues.apache.org/jira/browse/HDFS-12283
> Project: Hadoop HDFS
> Issue Type: Sub-task
> Components: ozone, scm
> Reporter: Weiwei Yang
> Assignee: Yuanbo Liu
> Attachments: HDFS-12283.001.patch, HDFS-12283-HDFS-7240.001.patch,
> HDFS-12283-HDFS-7240.002.patch, HDFS-12283-HDFS-7240.003.patch,
> HDFS-12283-HDFS-7240.004.patch, HDFS-12283-HDFS-7240.005.patch,
> HDFS-12283-HDFS-7240.006.patch
>
>
> The DeletedBlockLog is a persisted log in SCM to keep tracking container
> blocks which are under deletion. It maintains info about under-deletion
> container blocks that notified by KSM, and the state how it is processed. We
> can use RocksDB to implement the 1st version of the log, the schema looks like
> ||TxID||ContainerName||Block List||ProcessedCount||
> |0|c1|b1,b2,b3|0|
> |1|c2|b1|3|
> |2|c2|b2, b3|-1|
> Some explanations
> # TxID is an incremental long value transaction ID for ONE container and
> multiple blocks
> # Container name is the name of the container
> # Block list is a list of block IDs
> # ProcessedCount is the number of times SCM has sent this record to datanode,
> it represents the "state" of the transaction, it is in range of \[-1, 5\], -1
> means the transaction eventually failed after some retries, 5 is the max
> number times of retries.
> We need to define {{DeletedBlockLog}} as an interface and implement this with
> RocksDB {{MetadataStore}} as the first version.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
---------------------------------------------------------------------
To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org
