[
https://issues.apache.org/jira/browse/BOOKKEEPER-564?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13642961#comment-13642961
]
Ivan Kelly commented on BOOKKEEPER-564:
---------------------------------------
{quote}
a new patch to remove circular dependency by make the ledger storage interface
as CheckPoint LedgerStorage#checkpoint(CheckPoint). The return value indicates
the actual point that the ledger storage already synced all the data. keep the
original sync thread.
{quote}
The patch does not remove the circular dependency. It's still there in the same
form. There's no way to implement Checkpointer, without something like
{code}
Checkpointer cpkt = Checkpointer();
LedgerStorage ledgerStorage = new InterleavedLedgerStorage(cpkt);
cpkt.setLedgerStorage(ledgerStorage);
{code}
This patch doesn't explicitly do this, because SyncThread is an innerclass of
Bookie, but implicitly it's the exact same thing. Try making SyncThread a
static inner class and see what happens. Whats more, you don't even need the
circular dependency in your patch. SyncThread#newCheckpoint calls
journal#requestCheckpoint. You could simply have Journal implement
Checkpointer. But by now, the name Checkpointer doesn't make sense anymore, as
you aren't able to trigger a checkpoint action through the interface, so maybe
rename it to CheckpointSource. And if Journal is implementing this, you may as
well add checkpointComlete(Checkpoint) to CheckpointSource. And remove
Checkpoint#checkpointComplete(). So the SyncThread can call
journal.checkpointComplete(checkpoint), and it's clearer where the operation is
occurring. Checkpoint#checkpointComplete() looks a lot like action at a
distance.
So -1 For this patch.
{quote}
1) we don't need to rewrite sync thread, if we provide a new ledger storage.
2) syncing when entry logger rotated is a tradeoff between recovery time and
performance. if in future, we want to reduce recovery time and keep
performance, it is easy to do it.
{quote}
This can be achieved by breaking SyncThread into it's own class, and having the
ledger storage own it.
{quote}
bq. Journal implements CheckpointListener#checkpointOccurred(CheckPoint) which
moves the "lastmark" pointer.
it doesn't make any sense to make ledger storage control journal to move the
mark.
{quote}
Agreed, this is what I've been arguing for weeks. But I would also go further
and say it makes no sense for the journal to move the mark. There's are 3
components in checkpointing, the ledger storage, the journal and the checkpoint
mark. The checkpoint mark is relates to both the ledger storage and the
journal. Marks are created by the journal, and refer to the journal position,
but the decision of which mark is the checkpoint mark is taken by the ledger
storage. This is why I argued that it should not be stored in the journal but
in the ledger storage. Equally it could be separate component completely. But
since you argued it should be in the journal, and since it's a fact that the
ledger storage decides which mark is the checkpoint mark, I added this
interface.
> 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, 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
