hadoop-hdfs-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Aaron T. Myers (JIRA)" <j...@apache.org>
Subject [jira] [Updated] (HDFS-5138) Support HDFS upgrade in HA
Date Fri, 10 Jan 2014 09:02:11 GMT

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

Aaron T. Myers updated HDFS-5138:
---------------------------------

    Attachment: HDFS-5138.patch

Thanks a lot for the review, Todd. Here's an updated patch which should address all of your
feedback. Please have another look. Detailed responses below.

bq. thanks for the description in the above JIRA comment. Can you transfer this comment somewhere
into the docs, perhaps hadoop-hdfs-project/hadoop-hdfs/src/site/apt/HDFSHighAvailabilityWithQJM.apt.vm
or a new page? Perhaps with a slightly more user-facing angle.

Good thinking. Done.

bq. what would happen if the admin called finalizeUpgrade() when neither node had yet transitioned
to active? I don't see any sanity check here.. is it possible you'd end up leaving the shared
in an orphaned "upgrading" state and never end up finalizing it?

Great point. I've now added support for running finalizeUpgrade() on each NN from DFSAdmin,
and made the DFSAdmin command also check to ensure that one NN is indeed active before it
will finalize the upgrade.

bq. Similarly, what happens if you start one NN with -upgrade, and you start the other one
without -upgrade. It seems to me it should check for the upgrade lock file in the shared dir
and say "looks like an upgrade is in progress, please start the SBN with -upgrade".

Good thinking. Fixed.

bq. there are a few TODOs in the code that probably need to be addressed - nothing big, just
a few things you may have missed.

Yes, as I said, this was a prelim patch to make sure folks were OK with the direction. All
of them are pretty trivial and have been addressed in the latest patch, except for this one,
which I'd love to hear your thoughts on:
{code}
+    // TODO(atm): What should we do here in the case of multiple shared edits
+    // dirs, which isn't actually possible at the moment?
{code}
My suggestion is that until we do actually support having multiple shared edits dirs (perhaps
never) that we should just assert that there is in fact only a single one here.

bq. would be good to add Javadoc on the new methods, so that JM implementors know what the
upgrade process looks like. i.e what is pre-upgrade, etc?

Yep, good thinking. Done. Please have a look at the new javadocs.

bq. "Could not perform upgrade or more JournalNodes" error message has some missing words
in it.

Eagle-eye Lipcon. Good catch. Done.

bq. this line should be unreachable, right? maybe an AssertionError("Unreachable code") would
make more sense? Also this same exception message is used down below in canRollBack which
isn't quite right.

Good thinking. Done.

bq. when you upgrade the journal, I'd think you'd to copy over all the data from the PersistentLongFiles
into the new dir?

Good catch. Done. I've also updated the tests to ensure that this happens during upgrade of
JNs.

bq. worth considering a protobuf for the shared log lock, in case we want to add other fields
to it later (instead of the custom serialization you do now)

I thought about this, but concluded that with just two fields it probably wasn't worth it.
Do you feel strongly about this?

bq. need try...finally around the code where you write the shared log lock. On the read side
you're also forgetting to close it.

Good catch. Fixed.

bq. the creation of the shared log lock file is non-atomic... I'm worried that we may hit
the race in practice, since the AtomicFileOutputStream implies an fsync, which means that
between the exists() check and the rename to the lock file, you may really have a decently
long time window. Maybe we can use locking code like Storage does? Feel free to punt to a
follow-up.

Note that in the case the lock file is being created on a JN, that the creation of the shared
lock file is within a synchronized section. I agree that this could still be an issue in the
NFS case, but seems to me like we can reasonably punt that to a later JIRA.

bq. can you add a doc on doUpgradeOfSharedLogOnTransitionToActive()?

Yep, done.

bq. why are some of the functions package-private and others are public?

That was an error. You're right that the one that was public can be package-private. Fixed.

bq. make it an abstract class or give it a private constructor so it can't be instantiated,
since it's just static methods

Done. Made it abstract.

bq. brief javadocs would be nice for these methods, even though they're straight refactors
of existing code.

Sure. Added.

bq. in canRollBack(), you throw an exception if there is no shared log. That doesn't seem
right...

It actually is correct, since we only ever call this method in the event that HA is enabled,
so there really should be a shared log. In the event that HA is not enabled, the upgrade of
the edits dirs will be done directly on the storage dirs, instead of going through the edit
log upgrade code path. The same is true FSEditLog#doPreUpgrade, doUpgrade, doFinalize, etc.
I've changed the names of these methods to have "OfSharedLog" at the end to make this clear.

bq. capitalization of "RollBack" vs "Rollback" is a little inconsistent. Looks like Rollback
is consistently used prior to this patch, so probably best to stick with that.

I attempted to consistenly use "Rollback" (one word) when it's used as a noun, and "RollBack"
(two words) when it's used as a verb, which I believe to be grammatically correct. Perhaps
it's best to stick with just one capitalization scheme for method/variable names, for consistency,
even if it is technically incorrect English. Do you have an opinion here?

bq. in the switch statement on the startup option, I think you should keep the ROLLBACK case,
but just have it throw AssertionError – just to make sure we don't accidentally have some
case where we're passing it there but shouldn't be.GA

Good thinking. Done.

> Support HDFS upgrade in HA
> --------------------------
>
>                 Key: HDFS-5138
>                 URL: https://issues.apache.org/jira/browse/HDFS-5138
>             Project: Hadoop HDFS
>          Issue Type: Bug
>    Affects Versions: 2.1.1-beta
>            Reporter: Kihwal Lee
>            Assignee: Aaron T. Myers
>            Priority: Blocker
>         Attachments: HDFS-5138.patch, HDFS-5138.patch, HDFS-5138.patch, HDFS-5138.patch
>
>
> With HA enabled, NN wo't start with "-upgrade". Since there has been a layout version
change between 2.0.x and 2.1.x, starting NN in upgrade mode was necessary when deploying 2.1.x
to an existing 2.0.x cluster. But the only way to get around this was to disable HA and upgrade.

> The NN and the cluster cannot be flipped back to HA until the upgrade is finalized. If
HA is disabled only on NN for layout upgrade and HA is turned back on without involving DNs,
things will work, but finaliizeUpgrade won't work (the NN is in HA and it cannot be in upgrade
mode) and DN's upgrade snapshots won't get removed.
> We will need a different ways of doing layout upgrade and upgrade snapshot.  I am marking
this as a 2.1.1-beta blocker based on feedback from others.  If there is a reasonable workaround
that does not increase maintenance window greatly, we can lower its priority from blocker
to critical.



--
This message was sent by Atlassian JIRA
(v6.1.5#6160)

Mime
View raw message