You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hbase.apache.org by Lars George <la...@gmail.com> on 2010/01/29 14:17:55 UTC
HLog sync() questions
Hi JD,
I have more question in regards to HLog and more concretely the
underlaying SequenceFileLogWriter. This is what is called from HLog
(assuming that is current, you said you changed things to hflush() in
the replication context?):
@Override
public void sync() throws IOException {
this.writer.sync();
if (this.writer_out != null) {
this.writer_out.sync();
}
}
The first sync calls SequenceFile.Writer.sync() which is not at all
what we want, i.e. the hflush or flush/sync in general but setting a
sync marker in the file. What purpose does that have and why is that
needed here?
The second call is the deprecated FSDataOutputStream.sync() which does
the actual hflush() internally. Just wondering what the sync is for
really.
Looking at the old 0.20 call
public void sync() throws IOException {
lastLogFlushTime = System.currentTimeMillis();
if (this.append && syncfs != null) {
try {
this.syncfs.invoke(this.writer, NO_ARGS);
} catch (Exception e) {
throw new IOException("Reflection", e);
}
} else {
this.writer.sync();
}
this.unflushedEntries.set(0);
syncTime += System.currentTimeMillis() - lastLogFlushTime;
syncOps++;
}
That is calling thew actual syncfs aka hflush if available otherwise
also the above sync() on SequenceFile.Writer, which did and does this
(as noted above):
/** create a sync point */
public void sync() throws IOException {
if (sync != null && lastSyncPos != out.getPos()) {
out.writeInt(SYNC_ESCAPE); // mark the start of the sync
out.write(sync); // write sync
lastSyncPos = out.getPos(); // update lastSyncPos
}
}
Well, what that does I can see in the
SequenceFile.Reader.readRecordLength(). But that certainly does no OS
sync (or fsync or syncfs) nor hflush nor even a stream flush.
Why are we calling it? It is called internally anyways after every few
hundred bytes it says here
synchronized void checkAndWriteSync() throws IOException {
if (sync != null &&
out.getPos() >= lastSyncPos+SYNC_INTERVAL) { // time to emit sync
sync();
}
}
Just when a KV is say 10KB then it is basically in between nearly
every KV (unless the HLogKey + KeyValue is minute).
So overall I think we should *not* call writer.sync() unless it is
Syncable and then call hflush() instead?
Lars
Re: HLog sync() questions
Posted by Lars George <la...@gmail.com>.
On that note, should we make HLog.Writer also implement Syncable to
pass on the hsync() and hflush() calls? That would be in trunk
obviously.
On Fri, Jan 29, 2010 at 2:17 PM, Lars George <la...@gmail.com> wrote:
> Hi JD,
>
> I have more question in regards to HLog and more concretely the
> underlaying SequenceFileLogWriter. This is what is called from HLog
> (assuming that is current, you said you changed things to hflush() in
> the replication context?):
>
> @Override
> public void sync() throws IOException {
> this.writer.sync();
> if (this.writer_out != null) {
> this.writer_out.sync();
> }
> }
>
> The first sync calls SequenceFile.Writer.sync() which is not at all
> what we want, i.e. the hflush or flush/sync in general but setting a
> sync marker in the file. What purpose does that have and why is that
> needed here?
>
> The second call is the deprecated FSDataOutputStream.sync() which does
> the actual hflush() internally. Just wondering what the sync is for
> really.
>
> Looking at the old 0.20 call
>
> public void sync() throws IOException {
> lastLogFlushTime = System.currentTimeMillis();
> if (this.append && syncfs != null) {
> try {
> this.syncfs.invoke(this.writer, NO_ARGS);
> } catch (Exception e) {
> throw new IOException("Reflection", e);
> }
> } else {
> this.writer.sync();
> }
> this.unflushedEntries.set(0);
> syncTime += System.currentTimeMillis() - lastLogFlushTime;
> syncOps++;
> }
>
> That is calling thew actual syncfs aka hflush if available otherwise
> also the above sync() on SequenceFile.Writer, which did and does this
> (as noted above):
>
> /** create a sync point */
> public void sync() throws IOException {
> if (sync != null && lastSyncPos != out.getPos()) {
> out.writeInt(SYNC_ESCAPE); // mark the start of the sync
> out.write(sync); // write sync
> lastSyncPos = out.getPos(); // update lastSyncPos
> }
> }
>
> Well, what that does I can see in the
> SequenceFile.Reader.readRecordLength(). But that certainly does no OS
> sync (or fsync or syncfs) nor hflush nor even a stream flush.
>
> Why are we calling it? It is called internally anyways after every few
> hundred bytes it says here
>
> synchronized void checkAndWriteSync() throws IOException {
> if (sync != null &&
> out.getPos() >= lastSyncPos+SYNC_INTERVAL) { // time to emit sync
> sync();
> }
> }
>
> Just when a KV is say 10KB then it is basically in between nearly
> every KV (unless the HLogKey + KeyValue is minute).
>
> So overall I think we should *not* call writer.sync() unless it is
> Syncable and then call hflush() instead?
>
> Lars
>
Re: HLog sync() questions
Posted by Stack <st...@duboce.net>.
On Sat, Jan 30, 2010 at 6:10 AM, Lars George <la...@gmail.com> wrote:
>
> So with making Writer implement Syncable, does that make sense? Since
> that is what we really want, have HLog call the same functions on
> Writer for hsync() and hflush(). Right? That would enforce the right
> interface on implementing Writer classes. Just wondering.
>
That makes sense to me.
Stack
Re: HLog sync() questions
Posted by Lars George <la...@gmail.com>.
Hi Stack,
Comments inline below
Lars
On Fri, Jan 29, 2010 at 9:30 PM, Stack <st...@duboce.net> wrote:
> On Fri, Jan 29, 2010 at 5:17 AM, Lars George <la...@gmail.com> wrote:
>> @Override
>> public void sync() throws IOException {
>> this.writer.sync();
>> if (this.writer_out != null) {
>> this.writer_out.sync();
>> }
>> }
>>
>> The first sync calls SequenceFile.Writer.sync() which is not at all
>> what we want, i.e. the hflush or flush/sync in general but setting a
>> sync marker in the file. What purpose does that have and why is that
>> needed here?
>
> Yes. SF.sync does not what you'd expect a sync to do, as you point out
> later in your mail.
>
> We continue to call it in TRUNK because its what we've always done and
> my thinking was, maybe the marker written to SF file will be of use.
> IIUC the marker is used to pick up the parse after we've crossed the
> corrupted section.
Makes sense. Also makes sense to think about a better suited file
format anyways but that is already underway, so this is more for
understanding the legacy code.
>> The second call is the deprecated FSDataOutputStream.sync() which does
>> the actual hflush() internally. Just wondering what the sync is for
>> really.
>
> Yeah, the call to FSDOS.sync is the important one. Its deprecated but
> its doing the right thing (At the time of writing this was all that
> was available).
Understood.
>>
>> Looking at the old 0.20 call
>>
>> public void sync() throws IOException {
>> lastLogFlushTime = System.currentTimeMillis();
>> if (this.append && syncfs != null) {
>> try {
>> this.syncfs.invoke(this.writer, NO_ARGS);
>> } catch (Exception e) {
>> throw new IOException("Reflection", e);
>> }
>> } else {
>> this.writer.sync();
>> }
>> this.unflushedEntries.set(0);
>> syncTime += System.currentTimeMillis() - lastLogFlushTime;
>> syncOps++;
>> }
>>
>> That is calling thew actual syncfs aka hflush
>
> No. syncfs is hdfs-200 thing. hflush is hdfs-265. In 0.20, we had
> it rigged so that if hdfs-200 was present, we'd notice it and call
> syncFs on any sync invocation.
>
Yes, understood as well, I meant it called as you say syncfs, now we
call hflush.
>> Why are we calling it? It is called internally anyways after every few
>> hundred bytes it says here
>>
>> synchronized void checkAndWriteSync() throws IOException {
>> if (sync != null &&
>> out.getPos() >= lastSyncPos+SYNC_INTERVAL) { // time to emit sync
>> sync();
>> }
>> }
>>
>> Just when a KV is say 10KB then it is basically in between nearly
>> every KV (unless the HLogKey + KeyValue is minute).
>>
>
> I suppose it should come as no surprise that if actual SF sync is
> managed internally, it won't match KV boundaries. I think it may
> prove of use still though Lars letting the SF parse pick up again
> after its crossed a corrupt patch. I haven't checked the code though
> and maybe we want to fail if SF has any corruption in it?
Yes, as per the above. Makes sense as long as we use SequenceFile.
> Thanks for digging in on this Lars.
> St.Ack
Still digging :)
So with making Writer implement Syncable, does that make sense? Since
that is what we really want, have HLog call the same functions on
Writer for hsync() and hflush(). Right? That would enforce the right
interface on implementing Writer classes. Just wondering.
Re: HLog sync() questions
Posted by Stack <st...@duboce.net>.
On Fri, Jan 29, 2010 at 5:17 AM, Lars George <la...@gmail.com> wrote:
> @Override
> public void sync() throws IOException {
> this.writer.sync();
> if (this.writer_out != null) {
> this.writer_out.sync();
> }
> }
>
> The first sync calls SequenceFile.Writer.sync() which is not at all
> what we want, i.e. the hflush or flush/sync in general but setting a
> sync marker in the file. What purpose does that have and why is that
> needed here?
Yes. SF.sync does not what you'd expect a sync to do, as you point out
later in your mail.
We continue to call it in TRUNK because its what we've always done and
my thinking was, maybe the marker written to SF file will be of use.
IIUC the marker is used to pick up the parse after we've crossed the
corrupted section.
> The second call is the deprecated FSDataOutputStream.sync() which does
> the actual hflush() internally. Just wondering what the sync is for
> really.
Yeah, the call to FSDOS.sync is the important one. Its deprecated but
its doing the right thing (At the time of writing this was all that
was available).
>
> Looking at the old 0.20 call
>
> public void sync() throws IOException {
> lastLogFlushTime = System.currentTimeMillis();
> if (this.append && syncfs != null) {
> try {
> this.syncfs.invoke(this.writer, NO_ARGS);
> } catch (Exception e) {
> throw new IOException("Reflection", e);
> }
> } else {
> this.writer.sync();
> }
> this.unflushedEntries.set(0);
> syncTime += System.currentTimeMillis() - lastLogFlushTime;
> syncOps++;
> }
>
> That is calling thew actual syncfs aka hflush
No. syncfs is hdfs-200 thing. hflush is hdfs-265. In 0.20, we had
it rigged so that if hdfs-200 was present, we'd notice it and call
syncFs on any sync invocation.
> Why are we calling it? It is called internally anyways after every few
> hundred bytes it says here
>
> synchronized void checkAndWriteSync() throws IOException {
> if (sync != null &&
> out.getPos() >= lastSyncPos+SYNC_INTERVAL) { // time to emit sync
> sync();
> }
> }
>
> Just when a KV is say 10KB then it is basically in between nearly
> every KV (unless the HLogKey + KeyValue is minute).
>
I suppose it should come as no surprise that if actual SF sync is
managed internally, it won't match KV boundaries. I think it may
prove of use still though Lars letting the SF parse pick up again
after its crossed a corrupt patch. I haven't checked the code though
and maybe we want to fail if SF has any corruption in it?
Thanks for digging in on this Lars.
St.Ack