[
https://issues.apache.org/jira/browse/BOOKKEEPER-564?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13640224#comment-13640224
]
Ivan Kelly commented on BOOKKEEPER-564:
---------------------------------------
I was thinking about this some more last night. There's no way of having the
checkpointer outside of LedgerStorage without having a circular dependency. To
demonstrate, if I wanted to implement a checkpointer, I would have to do
something like the following.
{code}
class MyCheckpointer extends CheckPointer {
public CheckPoint newCheckpoint() {
return generateCheckpointSomehow();
}
public void startCheckpoint(CheckPoint checkpoint) {
ledgerStorage.checkpoint(checkpoint);
}
}
LedgerStorage ledgerStorage = new InterleavedLedgerStorage(conf, dirs, new
MyCheckpointer());
{code}
There's a circular dependency here and there's no way to get rid of it. This
circular dependency also exists in the Sijie's patch from 9 Apr, though it's
hidden behind inner classes. This is just bad software design, and I'm going to
-1 any patch that I find with such antipatterns.
Whats more, any solution that has the syncthread and ledgerstorage as side by
side objects rather than ledgerstorage owning the syncthread will run into this.
However, we've been arguing this for weeks now, and we need to find some middle
ground. I realise my patch isn't going to go in as is either, due to the points
made earlier. Specifically, referring to Sijie's comments on the 23rd, for 1)
this can go either way, I prefer LogMark in ledger storage, or at least out of
Journal, but I can live with it. However, for 2) sync thread has to be owned by
ledger storage, otherwise we get the antipattern described at the start of the
comment.
So how about this...
We morph Checkpointer into the following interface
{code}
interface CheckpointListener {
public CheckPoint newCheckpoint();
public void checkpointOccurred(Checkpoint checkpoint);
}
{code}
Journal implements CheckpointListener. LedgerStorage takes a Checkpoint
listener on construction. SyncThread is broken out from Bookie, and
LedgerStorage constructs and owns it, but the implementation is separate from
any particular LedgerStorage implementation.
I won't start coding this until I get some agreement that this is an acceptable
way forward.
> Better checkpoint mechanism
> ---------------------------
>
> Key: BOOKKEEPER-564
> URL: https://issues.apache.org/jira/browse/BOOKKEEPER-564
> Project: Bookkeeper
> Issue Type: Improvement
> Components: bookkeeper-server
> Reporter: Sijie Guo
> Assignee: Sijie Guo
> Fix For: 4.3.0
>
> Attachments: 0001-BOOKKEEPER-564-Better-checkpoint-mechanism.patch,
> 0001-BOOKKEEPER-564-Better-checkpoint-mechanism.patch,
> 0002-BOOKKEEPER-564-Better-checkpoint-mechanism.patch, BOOKKEEPER-564.patch,
> BOOKKEEPER-564.patch
>
>
> Currently, SyncThread made a checkpoint too frequently, which affects
> performance. data is writing to entry logger file might be blocked by syncing
> same entry logger file, which affect bookie to achieve higher throughput. We
> could schedule checkpoint only when rotating an entry log file. so new
> incoming entries would be written to newer entry log file and old entry log
> file could be synced.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
