jackrabbit-oak-issues mailing list archives

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

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

Francesco Mari commented on OAK-4803:
-------------------------------------

bq. 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.

This comment makes very much sense, because it highlights an asymmetry between the client
and the server. To be honest, I prefer the way the client is written, without an explicit
reference to the {{FileStore}}. This highlights the fact that the client is concerned with
sending requests and parsing responses from the server, instead of taking care of how the
data is actually used. I could have achieved the same separation of concerns in the server
as well by introducing a very small interface. In the end, these are the only three lines
where the {{FileStore}} is used in the server, which already suggests that this separation
of concerns exists - at least at the level of the handlers.

{noformat}
p.addLast(new GetHeadRequestHandler(new DefaultStandbyHeadReader(store)));
p.addLast(new GetSegmentRequestHandler(new DefaultStandbySegmentReader(store)));
p.addLast(new GetBlobRequestHandler(new DefaultStandbyBlobReader(store)));
{noformat}

Is this maybe the scope of another improvement issue?

bq. 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.

I quite don't agree here. You need a client to perform a sync. The client could be used for
different purposes other than the sync, so it makes sense of having the sync process depending
on a client and not the other way around. Anyway, the lines you pointed out are leftovers
from the refactoring and I recognise that they are confusing. I will clean them up.

bq. One minor change in {{StandbySync.run()}}, to allow the state to actually enter {{STATUS_STARTING}}.

Good catch. It has to be cleaned up as part of this patch.

bq. 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.

Nice suggestion. That method should have a more appropriate name, and I like your proposal.

> 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