[
https://issues.apache.org/jira/browse/OAK-4803?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15492953#comment-15492953
]
Andrei Dulceanu edited comment on OAK-4803 at 9/15/16 10:26 AM:
----------------------------------------------------------------
[~frm], here are some of my observations regarding the patch:
# When looking at {{StandbyClient}} and {{StandbyServer}} I find it a little
bit odd that the former doesn't have a {{FileStore}}, while the latter has.
IMHO having a {{FileStore}} reference in the client would make sense and would
also help reading the code, as it is much clearer from the beginning who owns
what.
# Along the same lines, IMHO it would make sense to reverse the relationship
between {{StandbyClient}} and {{StandbySync}} since it looks more natural to
have a client which wants to perform a sync as opposed to have a sync which
will create a client, assign it a file store and then execute the sync. For
example, replacing the line
{code:java}
StandbyClient cl = newStandbyClient(secondary);
{code}
with this line
{code:java}
StandbySync cl = newStandbyClient(secondary);
{code}
seems a little bit confusing to me.
# One minor change in {{StandbySync.run()}}, to allow the state to actually
enter {{STATUS_STARTING}}:
{code:java}
state = STATUS_STARTING;
synchronized (sync) {
if (active) {
return;
}
state = STATUS_RUNNING;
active = true;
}
{code}
# another minor change in {{StandbySyncExecution}}: rename
{{copySegmentFromPrimary}} to {{copySegmentHierarchyFromPrimary}} or any other
explanatory method name, since this method does a BFS starting with the initial
segment to fetch from server.
/cc [~marett]
was (Author: dulceanu):
@frm, here are some of my observations regarding the patch:
# When looking at {{StandbyClient}} and {{StandbyServer}} I find it a little
bit odd that the former doesn't have a {{FileStore}}, while the latter has.
IMHO having a {{FileStore}} reference in the client would make sense and would
also help reading the code, as it is much clearer from the beginning who owns
what.
# Along the same lines, IMHO it would make sense to reverse the relationship
between {{StandbyClient}} and {{StandbySync}} since it looks more natural to
have a client which wants to perform a sync as opposed to have a sync which
will create a client, assign it a file store and then execute the sync. For
example, replacing the line
{code:java}
StandbyClient cl = newStandbyClient(secondary);
{code}
with this line
{code:java}
StandbySync cl = newStandbyClient(secondary);
{code}
seems a little bit confusing to me.
# One minor change in {{StandbySync.run()}}, to allow the state to actually
enter {{STATUS_STARTING}}:
{code:java}
state = STATUS_STARTING;
synchronized (sync) {
if (active) {
return;
}
state = STATUS_RUNNING;
active = true;
}
{code}
# another minor change in {{StandbySyncExecution}}: rename
{{copySegmentFromPrimary}} to {{copySegmentHierarchyFromPrimary}} or any other
explanatory method name, since this method does a BFS starting with the initial
segment to fetch from server.
/cc [~marett]
> Simplify the client side of the cold standby
> --------------------------------------------
>
> Key: OAK-4803
> URL: https://issues.apache.org/jira/browse/OAK-4803
> Project: Jackrabbit Oak
> Issue Type: Improvement
> Components: segment-tar
> Reporter: Francesco Mari
> Assignee: Francesco Mari
> Fix For: Segment Tar 0.0.12
>
> Attachments: OAK-4803-01.patch
>
>
> The implementation of the cold standby client is overly and unnecessarily
> complicated. It would be way clearer to separate the client code in two major
> components: a simple client responsible for sending messages to and receive
> responses from the standby server, and the synchronization algorithm used to
> read data from the server and to save read data in the local {{FileStore}}.
> Moreover, the client simple client could be further modularised by
> encapsulating request encoding, response decoding and message handling into
> their own Netty handlers.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
