hadoop-hdfs-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Todd Lipcon (JIRA)" <j...@apache.org>
Subject [jira] Updated: (HDFS-1521) Persist transaction ID on disk between NN restarts
Date Thu, 16 Dec 2010 22:37:01 GMT

     [ https://issues.apache.org/jira/browse/HDFS-1521?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel

Todd Lipcon updated HDFS-1521:

    Attachment: hdfs-1521.5.txt

Thanks for the thorough review. Here's a new patch.

bq. FSImage.loadFSEdits(StorageDirectory sd) should return boolean instead of FSEditLogLoader

bq. You can avoid introducing FSEditLogLoader.needResave by setting expectedStartingTxId before
checking that logVersion != FSConstants.LAYOUT_VERSION. Then the old logic of counting this
event as an extra transaction will work

I found the former logic here to be very confusing and somewhat of a hack. It's also important
that the loader returns the correct number of edits rather than potentially returning 1 when
there are 0 edits. If it did that, it would break many cases by potentially causing a skip
in transaction IDs. Though the new code adds a new member, the new member has a clear purpose
and I think it's easier to understand from the caller's perspective, especially now that your
point #1 above is addressed.

bq. It would be good if you could replace FSEditLogLoader.expectedStartingTxId member by the
respective parameter to loadFSEdits
bq. I think after that you can also get rid of FSEditLogLoader.numEditsLoaded.

bq. Why don't we write first opCode, then txID, then Writable. There will be less code changes
on the loading part
Very good call! This indeed cleaned up the loading code a lot.

bq. Should we introduce TransactionHeader at this point and write it as Writable. Just something
to consider
I think given that the header is still pretty simple it's not worth it at this point.

bq. Need to change JavaDoc for EditLogOutputStream.write(). Missing parameter

bq. I don't see any reason to have txID in the beginning of every edits file. You will have
it the name, right
bq. beginTransaction() instead of startTransaction, as it matches with endTransaction()

bq. Don't change rollEditLog() to return long. It is only used in the test
It's necessary that the transaction ID be returned inside the same synchronization block.
If we used a separate call to getLastWrittenTxId() then another txid could have been written
in between (note that the test is multithreaded).

bq. It looks to me that FSImage.checkpointTxId is simply currentTxId. If it is, it would be
more intuitive

It's not really current - it's the txid of the image file, not including any edits that have
been written to the edit log - sort of like how checkpointTime is set only when an image is
saved. Naming it "currentTxId" would imply that it is updated on every edit.

bq. BackupStorage.lastAppliedTxId isn't it just checkpointTxId, which is already defined in
the base FSImage.

Contrary to above, lastAppliedTxId refers to the transaction ID that has been applied to the
namespace. This is always >= checkpointTxId - checkpointTxId only changes when the BN saves
an image, but lastAppliedTxId changes every time some edits are applied via RPC.

I'll run the new patch through the unit test suite one more time.

> Persist transaction ID on disk between NN restarts
> --------------------------------------------------
>                 Key: HDFS-1521
>                 URL: https://issues.apache.org/jira/browse/HDFS-1521
>             Project: Hadoop HDFS
>          Issue Type: Sub-task
>          Components: name-node
>    Affects Versions: 0.22.0
>            Reporter: Todd Lipcon
>            Assignee: Todd Lipcon
>             Fix For: 0.22.0
>         Attachments: hdfs-1521.3.txt, hdfs-1521.4.txt, hdfs-1521.5.txt, hdfs-1521.txt,
> For HDFS-1073 and other future work, we'd like to have the concept of a transaction ID
that is persisted on disk with the image/edits. We already have this concept in the NameNode
but it resets to 0 on restart. We can also use this txid to replace the _checkpointTime_ field,
I believe.

This message is automatically generated by JIRA.
You can reply to this email to add a comment to the issue online.

View raw message