[
https://issues.apache.org/jira/browse/DERBY-3064?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12530407
]
Øystein Grøvlen commented on DERBY-3064:
----------------------------------------
Thanks for the patch, Narayanan. It looks good and is very well
documented. Except for my first three issues, my comments are minor:
1. I do not see the need for recording lastShippingTime. When a
forceFlush is made, I do not think you would want to delay the next
regular sending of log records since there will still be more log
to send after the forceFlush has sent one chunk. Rather, I think
you would want to notify the log shipping thread that it is time
for another send. Hence, I suggest that you drop testing for time,
and use wait() instead of sleep() so that it is possible to wake up
the thread while it is waiting.
2. It seems like shipALogChunk will send a message even if there is no
log records to send. Should not the sending also be part of the
body of the if statement?
3. I would think you need some way for the master controller to stop
log shipping (e.g., when replication is stopped). In other words,
the LogShipper interface need a stop method, and the loop needs to
test for whether it should stop.
4. Why are exceptions from shipALogChunk handled differently by run()
and forceFlush()? Maybe you could let shipALogChunk handle the
exceptions instead so you do not have to do it in two places?
5. I suggest dropping 'Replication' from the name of
'ReplicationAsynchronousLogShipper'. Would make it a bit shorter,
and I think it is evident that we are talking about replication
here.
6. The constructor is declared to throw a StandardException and the
javadoc says it may happen when you 'register to the shipping
daemon". Is this preparing for something that will be added later?
7. In both files there is a typo in the javadoc for flushedInstance
('latestInstanceFlishedToDisk').
8. For ReplicationAsynchronousLogShipper#flushedInstance, I think you
should include in the javadoc that calling it will have no effect.
> Implement the LogShipper that will enable the shipping of Log records from
> the master to the slave
> --------------------------------------------------------------------------------------------------
>
> Key: DERBY-3064
> URL: https://issues.apache.org/jira/browse/DERBY-3064
> Project: Derby
> Issue Type: Sub-task
> Reporter: V.Narayanan
> Assignee: V.Narayanan
> Attachments: LogShipperImpl_v1.diff, LogShipperImpl_v1.stat,
> LogShipperImpl_v2.diff, LogShipperImpl_v2.stat
>
>
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
