jackrabbit-oak-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Andrei Dulceanu (JIRA)" <j...@apache.org>
Subject [jira] [Comment Edited] (OAK-4803) Simplify the client side of the cold standby
Date Thu, 15 Sep 2016 10:27:20 GMT

    [ 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)

Mime
View raw message