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