hadoop-hdfs-issues mailing list archives

[jira] [Commented] (HDFS-2149) Move EditLogOp serialisation and deserialation into one place

Date

Wed, 20 Jul 2011 06:43:58 GMT

[ https://issues.apache.org/jira/browse/HDFS-2149?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13068177#comment-13068177
]
Todd Lipcon commented on HDFS-2149:
-----------------------------------
Looking over the patch, it seems like this is really two distinct parts:
1) cleans up the writing of edits:
-- rather than writing logEdit(opcode, Writable...) in FSEditLog, the API is logEdit(op record)
-- this API change filters down into EditLogOutputStream as well
-- writeRaw is introduced to be used from the BN (like in 1073 branch)
-- EditLogBackupOutputStream, which used to buffer Writables in JournalRecord, now directly
buffers the op objects
2) Changes the API around *reading* ops to be an iterator-style interface rather than a byte-style
interface, by pushing down the FSEditLogOp.Reader into EditLogInputStream.
I'm convinced that (1) above is a good idea, and I think the patch is mostly ready for commit
modulo a couple things below.
I'm not yet convinced that (2) above is quite right yet. We might need another iteration or
two to discuss the APIs.
Would you mind using this JIRA to get part (1) committed, and then work out the kinks in (2)
next? Or is there some reason that part (1) depends on part (2) that I'm missing?
Issues:
*EditLogBackupOutputStream bug*:
Buffering the Op objects in EditLogBackupOutputStream is likely to cause a bug since the objects
are reused. If a single thread logs twice before syncing (unlikely but possible for some operations)
the edits will get modified in place and the BN will diverge. This might be a bug in the current
BN as well, but the Writables weren't always reused, whereas these op objects are.
To fix this, I think we should make EditLogBackupOutputStream and EditLogFileOutputStream
act the same - ie serialize the ops immediately into a buffer. Perhaps they could share this
code in the abstract base class? Or, introduce a new classs like JournalDoubleBuffer?
*getInstance for AddCloseOp*:
Rather than have two separate ThreadLocals for AddCloseOp, I think it would be cleaner to
add a "setOp()" function there on a single instance. My only reasoning is that then all of
the classes are nicely symmetric.
*javadoc on writeRaw*:
I find this javadoc confusing. What do you mean by "should only be used in new code"? Also,
the punctuation is incomplete which makes the comment hard to read. Perhaps just copy the
javadoc from the 1073 branch s it doesn't need to be resolved at merge time?
*For brevity*:
Here's a thought: rather than giving each op its own ThreadLocal and getInstance method, why
not make the {{opInstances}} EnumMap that we already have into a ThreadLocal? Something like:
{code}
// in FSEditLogOp.java:
ThreadLocal<EnumMap<FSEditLogOpCodes, FSEditLogOp>> opInstances =
new ThreadLocal<EnumMap<FSEditLogOpCodes, FSEditLogOp>>() {
@Override
protected EnumMap<FSEditLogOpCodes, FSEditLogOp> initialValue() {
// ...
}
};
<T extends FSEditLogOp> T getThreadLocalOp(
FSEditLogOpCodes opCode, Class<T> clazz) {
return clazz.cast(opInstances.get().get(opCode));
}
// in FSEditLog.java:
public void logCloseFile(String path, INodeFile newNode) {
AddCloseOp op = FSEditLogOp.getThreadLocalOp(OP_CLOSE, AddCloseOp.class)
.setPath(path)
...;
}
{code}
I think this would reduce the amount of boilerplate and shouldn't be any discernible perf
difference.
> Move EditLogOp serialisation and deserialation into one place
> -------------------------------------------------------------
>
> Key: HDFS-2149
> URL: https://issues.apache.org/jira/browse/HDFS-2149
> Project: Hadoop HDFS
> Issue Type: Sub-task
> Reporter: Ivan Kelly
> Assignee: Ivan Kelly
> Fix For: 0.23.0
>
> Attachments: HDFS-2149.diff, HDFS-2149.diff
>
>
> On trunk serialisation of editlog ops is in FSEditLog#log* and deserialisation is in
FSEditLogOp.*Op . This improvement is to move the serialisation code into one place, i.e under
FSEditLogOp.*Op.
> This is part of HDFS-1580.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira