[
https://issues.apache.org/jira/browse/DERBY-3184?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12557221#action_12557221
]
oysteing edited comment on DERBY-3184 at 1/9/08 2:41 AM:
----------------------------------------------------------------
I have looked at derby-3184-2a.diff and the patch looks good. I do
not have any major issues. However, I am not able understand the
intention behind slavepremode, and this does not seem to be explained
anywhere either. Some documentation would be nice.
Some minor comments that you may consider for a follow-up patch:
SlaveDatabase:
1. The 'be' in item 4 of class javadoc should probably be removed.
2. Why not use javadoc style for comment for inReplicationSlaveMode?
3. I think it is a bit confusing that inReplicationSlaveMode is
initialized both in declaration and in boot.
4. canSupport() lacks javadoc for parameter
5. Instead of making create and startParams volatile members,
why not pass it to a SlaveDatabaseThread constructor?
6. Did you consider making SlaveDatabaseBootThread a Runnable
instead of a Thread? It does not seem like your run method depend
on inheriting any behavior from the Thread class.
7. The call to currentThread() is not necessary since sleep is a
static member of Thread.
SlaveFactory:
8. The import of Property is no longer necessary
9. Some explanation of SLAVE_PRE_MODE would be good.
LogToFile:
10. (Not introduced by this patch) initializeReplicationSlaveRole()
javadoc violate the rule that the first sentence of a javadoc
should summarize the purpose of the method.
was (Author: oysteing):
I have looked at derby-3184-2a.diff and the patch looks good. I do
not have any major issues. However, I am not able understand the
intention behind slavepremode, and this does not seem to be explained
anywhere either. Some documentation would be nice.
Some minor comments that you may consider for a follow-up patch:
SlaveDatabase:
1. The 'be' item 4 of class javadoc should probably be removed.
2. Why not use javadoc style for comment for inReplicationSlaveMode?
3. I think it is a bit confusing that inReplicationSlaveMode is
initialized both in declaration and in boot.
4. canSupport() lacks javadoc for parameter
5. Instead of making create and startParams volatile members,
why not pass it to a SlaveDatabaseThread constructor?
6. Did you consider making SlaveDatabaseBootThread a Runnable
instead of a Thread? It does not seem like your run method depend
on inheriting any behavior from the Thread class.
7. The call to currentThread() is not necessary since sleep is a
static member of Thread.
SlaveFactory:
8. The import of Property is no longer necessary
9. Some explanation of SLAVE_PRE_MODE would be good.
LogToFile:
10. (Not introduced by this patch) initializeReplicationSlaveRole()
javadoc violate the rule that the first sentence of a javadoc
should summarize the purpose of the method.
> Replication: Connection attempts to a database in slave mode must fail
> ----------------------------------------------------------------------
>
> Key: DERBY-3184
> URL: https://issues.apache.org/jira/browse/DERBY-3184
> Project: Derby
> Issue Type: Sub-task
> Components: Services
> Affects Versions: 10.4.0.0
> Reporter: Jørgen Løland
> Assignee: Jørgen Løland
> Attachments: derby-3184-1.diff, derby-3184-1.stat,
> derby-3184-2a.diff, derby-3184-2a.stat, derby-3184-bugfix-1a.diff,
> derby-3184-bugfix-1a.stat
>
>
> When a database 'X' is booted in slave mode, the call to
> Monitor.startPersistentService("X",...) will not complete because the call
> gets stuck in LogToFile#recover. This is intentional.
> For as long as startPersistentService is blocked, however, other threads that
> try to connect to 'X' (effectively calling Monitor.findService("X", ...))
> will also hang. This is unacceptable, and needs to raise an error.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
