[ 
https://issues.apache.org/jira/browse/OAK-4803?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15492953#comment-15492953
 ] 

Andrei Dulceanu commented on OAK-4803:
--------------------------------------

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

Reply via email to