[jira] Commented: (DERBY-3184) Replication: Connection attempts to a database in slave mode must fail
[
https://issues.apache.org/jira/browse/DERBY-3184?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12549407
]
Dyre Tjeldvoll commented on DERBY-3184:
---
The patch looks good, and all tests pass for me, +1.
However, I'm not very familiar with the code being modified (the
Monitor/Service system), so I'll wait a little longer before I commit it, just
to give others a chance to get their comments in.
> 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
>
>
> 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.
[jira] Commented: (DERBY-3184) Replication: Connection attempts to a database in slave mode must fail
[
https://issues.apache.org/jira/browse/DERBY-3184?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12549456
]
Daniel John Debrunner commented on DERBY-3184:
--
> I must admit I feel a bit uneasy introducing knowledge about
> replication into the Monitor which currently does not seem to have any
> knowledge about databases (except something called a persistent
> service).
+1 to the uneasiness about adding replication knowledge to the monitor. I think
Derby (&Cloudscape) has benefited over the years from isolating components from
each other. Some future direction might to replace Derby's monitor with some
other existing open-source IoC system that has a cleaner design and by
definition will be separated from knowledge of Derby's code.
> 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
>
>
> 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.
[jira] Commented: (DERBY-3184) Replication: Connection attempts to a database in slave mode must fail
[
https://issues.apache.org/jira/browse/DERBY-3184?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12549416
]
Øystein Grøvlen commented on DERBY-3184:
Thanks for the patch, Jørgen.
I must admit I feel a bit uneasy introducing knowledge about
replication into the Monitor which currently does not seem to have any
knowledge about databases (except something called a persistent
service). However, I see that the current design would need a rewrite
to in order to be able to avoid this, since there is currently no way
of getting hold of a service where the booting has not completed.
Currently, if you try to find a service during booting, you will hang
waiting for the boot to complete.
I have not looked into the details, but it seems that it should be
possible to get a handle to a Database that is booting, through
BaseMonitor.findModule with some extra wiring in a static method in
Monitor. A Database object is definitely a better fit for replication
logic. However, I have not thought about the consequences of
providing such an access point. It would be useful if someone with
more experience with the Monitor framework, could provide a
recommendation here.
Given the approach of the current patch, I wonder whether it would be
better to isolate the "pollution" of replication logic that is
currently spread across the BaseMonitor and TopService classes, to
just the TopService class. It seems like all the necessary
information should be available there.
Some minor nits:
- Why are the added methods in TopService protected and not just
package private?
- Indentation in Monitor.findService looks a bit awkward due to
the line break before 'throws'.
> 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
>
>
> 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.
[jira] Commented: (DERBY-3184) Replication: Connection attempts to a database in slave mode must fail
[
https://issues.apache.org/jira/browse/DERBY-3184?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12550071
]
Jørgen Løland commented on DERBY-3184:
--
Øystein and Dan.
In light of your comments, I agree that this is not the right
solution to the problem. I have a new suggestion for how to solve
this without touching the monitor.
* Add a new implementation of Database, called something like
SlaveDatabase
* Modify canSupport of all instances of Database so that
SlaveDatabase is booted if slave replication is specified
* When booted, SlaveDatabase will spawn a new tread that calls
BasicDatabase#boot(). This thread will be blocked in
LogToFile#recover(). After spawning the thread,
SlaveDatabase#boot finishes. Calls to Monitor#findService will
return the SlaveDatabase object, but all method calls will
result in an SQL Exception with state
CANNOT_CONNECT_TO_DB_IN_SLAVE_MODE:
public LanguageConnectionContext setupConnection(..) {
if (inSlaveMode) {
throw StandardException (SQLState.CANNOT_CONNECT...)
}
return basicDatabase.setupConnection(...);
}
> 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
>
>
> 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.
[jira] Commented: (DERBY-3184) Replication: Connection attempts to a database in slave mode must fail
[
https://issues.apache.org/jira/browse/DERBY-3184?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=1231#action_1231
]
Jørgen Løland commented on DERBY-3184:
--
No problem. Thanks for reviewing and committing :)
> 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-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.
[jira] Commented: (DERBY-3184) Replication: Connection attempts to a database in slave mode must fail
[
https://issues.apache.org/jira/browse/DERBY-3184?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12557221#action_12557221
]
Øystein Grøvlen commented on DERBY-3184:
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.
[jira] Commented: (DERBY-3184) Replication: Connection attempts to a database in slave mode must fail
[
https://issues.apache.org/jira/browse/DERBY-3184?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12557239#action_12557239
]
Øystein Grøvlen commented on DERBY-3184:
Thinking a bit more about this patch, I have a few more
questions/comments:
11. Have you verified that there is either no way that the different
BasicDataBase methods can be called on a SlaveDataBase while in
slave mode, or that for those methods that may be called, no harm
may happen?
12. The more I think about it, the less sure I am about what you are
trying to achieve by making local_startParams volatile. I do not
think a volatile reference will synchronize access to the referred
instance. I see there might be issues with concurrent access to
UpdateServiceProperties.storageFactory, but I think that needs to
be solved in that class.
13. A sleep a two seconds in the boot method between checking for
StorageFactory is unnecessary long I think. I think a good rule
of thumb is to make it as short as possible without significantly
affecting the performance, and I think you can check a reference
for null more often than every other second without affecting
performance. (While a two-second delay of a boot, is probably not
an issue in production settings, there might be testing scenarios
with repeated booting of database where the total duration of the
test may be an issue.)
> 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.
[jira] Commented: (DERBY-3184) Replication: Connection attempts to a database in slave mode must fail
[
https://issues.apache.org/jira/browse/DERBY-3184?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12557948#action_12557948
]
Øystein Grøvlen commented on DERBY-3184:
I discovered the func. spec. needs to be updated to reflect this patch.
Currently, the func spec says:
"A valid connection to the database is returned to the caller of the startSlave
command."
I guess this is no longer true with the latest patch.
> 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-2b.diff,
> derby-3184-2b.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.
[jira] Commented: (DERBY-3184) Replication: Connection attempts to a database in slave mode must fail
[
https://issues.apache.org/jira/browse/DERBY-3184?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12558004#action_12558004
]
Øystein Grøvlen commented on DERBY-3184:
Jørgen Løland (JIRA) wrote:
> 5:
> AFAIK, inner classes don't have constructors, but I added a
> setParams method that does the same. Hence, removed local_*
I do not think this is true. The introductory example on inner classes
in Gosling et.al. 'Java Programming Language' (3rd edition) , shows
a non-static member class with constructor.
> 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-2b.diff,
> derby-3184-2b.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.
[jira] Commented: (DERBY-3184) Replication: Connection attempts to a database in slave mode must fail
[
https://issues.apache.org/jira/browse/DERBY-3184?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12561329#action_12561329
]
Øystein Grøvlen commented on DERBY-3184:
Error handling in SlaveDatabase looks good, but since ReplicationLogger does
not contain any state, I think it would be better to make logError static.
Then you would not have to instantiate in every class where you need error
handling.
It also worries me a bit that you had to import StandardException in
DRDAConnThread, but that is probably not your fault. It seems to me that
validateSecMecUSRSSBPWD() does things that maybe should have been handled
inside the database. Anyway, we should make sure to test this case. That is,
try to connect to a slave database using SecMecUSRSSBPWD.
A minor issue: In EmbedConnection: No need to case se, it is already a
StandardException.
> 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-2b.diff,
> derby-3184-2b.stat, derby-3184-2c.diff, derby-3184-3a.diff,
> derby-3184-3a.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.
