[#HADOOP-5743] Configuration: Hierachical; documented; null-safe; thread-safe

Hadoop Common

Configuration: Hierachical; documented; null-safe; thread-safe

Created: 24/Apr/09 07:27 PM Updated: 04/Jun/09 08:40 AM

Return to search

Component/s:

conf

Affects Version/s:

None

Fix Version/s:

None

Time Tracking:

Not Specified

File Attachments:

File Name	Date Attached	Attached By	Size
01_configuration.patch	2009-04-24 07:29 PM	Shevek	50 kB
02_configuration.patch	2009-04-24 08:48 PM	Shevek	53 kB

Issue Links:

Reference

This issue is related to:

~~HADOOP-5708~~ Configuration should provide a way to write only properties that have been set

Release Note:

Second draft will satisfy comments in 5708, but leaves questions outstanding.

Description

« Hide

Patch to configuration to:

Update documentation to match code
Make configurations hierarchical
Remove global REGISTRY of configurations
Adding global default does not reload child configurations
(Eventually) make properly thread-safe
Allow well-behaved overriding with nulls
Allow future satisfaction of ticket 5708 (with which I agree, but Doug does not).

All

Comments

Work Log

Change History

Subversion Commits

Sort Order:

[ Permlink | « Hide ]

Shevek added a comment - 24/Apr/09 07:28 PM

See 5708 for comments from Doug relating to first draft patch.

[ Permlink | « Hide ]

Shevek added a comment - 24/Apr/09 07:29 PM

First draft patch, comments from Doug in 5708 outstanding.

[ Permlink | « Hide ]

Shevek added a comment - 24/Apr/09 07:31 PM

Doug, is the configuration serialization format allowed to change (to allow on-wire preservation of hierarchy metadata, and thus runtime semantics of resources/null-override-safety/etc), or would that be a disallowed incompatibility?

[ Permlink | « Hide ]

Shevek added a comment - 24/Apr/09 08:48 PM

Change to wire format would also allow preservation of finalParameters, which is no longer broken with this patch.

Kinda needs review. Lots of questions in the code. A better synchronization structure rather requires a slightly different data model, where finalParameters is stored within the Map.

[ Permlink | « Hide ]

Doug Cutting added a comment - 24/Apr/09 09:38 PM

I don't know all of the places where Configuration is used as a Writable, but my hunch is that it's okay to change its format. Where it's passed in an RPC, an incompatible change is okay, since we don't permit cross-version RPC. Configurations are written to disk on the job tracker, but I don't think we guarantee that the job queue survives upgrades.

It might be nice is if it included a version number in the serialized form. The new version could be -1, so that old code reading the new format would see this as the size and fail. But new code reading the old could do something back-compatible when the version is positive.

There are still lots of whitespace-only changes in your patch that inject noise and make me stop reading it.

I'm also not (yet) highly incented to spend a lot of time reviewing this, since it's not entirely clear to me that this is fixing problems that folks are actually encountering. Radically changing something low-level like this has a very good chance of breaking lots of systems, which will take a lot of time to debug and fix, and should thus not be entered into lightly.

That said, if you feel strongly, I encourage you to develop this to the point where it passes unit tests and can be automatically tested by Hudson. Also, please read your patch files before you submit them to see that they only include changes that are critical to your mission. They should not include discussions of the changes annotated with your name. Jira is for discussion of patches.

http://wiki.apache.org/hadoop/HowToContribute