|
Attached patch 1a, which makes the original repro pass.
Thanks for the fix Kristian. I've verified that it works.
Should we also set colon = name.indexOf(':') in the body of the new if statement? The if statement right below it is supposed to return immediately if the subsubprotocol doesn't match, but since colon is still -1, that code is not executed. Not that it appears to cause any problems. Alternatively, and perhaps clearer, I think we could change the if statement like this: if (colon == -1 && !getType().equals(PersistentService.DIRECTORY)) { // no subsubprotocol specified, and this is not the default service return null; } Thanks for looking at the patch, Knut.
I'll look at your suggestions and test them. There are several asserts that might be triggered by changes to this code. I guess we should have had a variable like PersistentService.DEFAULT_PROTOCOL, but since we're not changing it it's not crucial. One use case could be to allow setting the default protocol to use the in-memory storage factory instead of the directory storage factory. This could be useful in scenarios where you want to use the in-memory back end for testing, but don't want to change existing code. Thanks for thinking about this issue, Knut and Kristian. What do you think the behavior should be:
1) The memory subprotocol defines a separate namespace of databases. In this situation, it should be possible to have two databases with the same name, one in the memory namespace and one in the namespace accessed by all other subprotocols or 2) The first thread to open a database defines which subprotocols must be used to access the database from other connections--until the database is brought down. If a database was first opened by the memory subprotocol, then a database by that name cannot be accessed simultaneously by any other subprotocol. Conversely, if a database was first opened by the default or classpath subprotocols, then a database by that name cannot be accessed simultaneously by the memory subprotocol. I have two worries: A) We may not understand the current behavior of our subprotocols well enough. See DERBY-4172. B) The code may assume that there is only one namespace for databases. Adding another namespace may be tricky. I think I would lean toward (2). If we later decide that (2) is too restrictive and we think we understand the code well enough, then we could migrate to (1) without causing serious compatibility issues. I think that going from (1) to (2) will have serious compatibility implications. One potential problem with (2) is that in a multi-user environment, one user can create an in-memory database with the same path as a known, unbooted disk-based database, and thereby prevent other users from booting the disk-based database.
Which compatibility issues would we see when moving from (1) to (2) that we won't see when moving from the current situation to (2)? Attached patch 1b, which will cause null to be returned if no subsubprotocol is specified and the storage factory isn't the default one.
This is what the JavaDoc for PersistentService.getCanonicalServiceName says: /** Convert a service name into its canonical form. Returns null if the name cannot be converted into a canonical form. @exception No canonical name, name probably invalid */ Returning null is in agreement with the following existing code (StorageFactoryService.getCanonicalServiceName): if( colon > 1) // Subsubprotocols must be at least 2 characters long { if( ! name.startsWith( protocolLeadIn)) return null; // It is not our database name = name.substring( colon + 1); } Since there's an ongoing discussion about database namespaces and subsubprotocols, I'll hold off the commit for a while. Committed patch 1b to trunk with revision 782954.
I'll backport it to 10.5 after the nightly tests have been run a few times. I think patch 1b broke something on Windows, I see errors in BasicInMemoryDbTest on all the Windows platforms. Not having a Windows machine to debug on, this may have to wait until tomorrow before I can debug the problem.
Committed patch 2a to trunk with revision 784682.
Awaiting further confirmation from the nightly tests before backporting to the 10.5 branch. The error on Windows platforms in the nightly tests is no longer happening.
Backported to 10.5 with revision 786043. Verified on trunk. Closing.
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
 |
|
Bug
Closed
Major
Connections to on-disk db go to in-memory db if in-memory db with same name is booted
String protocolLeadIn = getType() + ":";
int colon = name.indexOf( ':');
if( colon > 1) // Subsubprotocols must be at least 2 characters long
{
if( ! name.startsWith( protocolLeadIn))
return null; // It is not our database
name = name.substring( colon + 1);
}
if( getType().equals( PersistentService.DIRECTORY)) // The default subsubprototcol
protocolLeadIn = "";
final String nm = name;
If there is no subsubprotocol present in the name, the code above will use the one from the storage factory being checked. In the case of VFMemoryStorageFactory, getType() returns 'memory'.
If you connect to the on-disk db first, than in-memory and finally on-disk again, the order of the services will be different and you'll (correctly) get a connection to the on-disk db.
Adding the following lines of code after 'int colon = ...' fixes the original repro:
// Add the default subsubprotocol if there is none present.
if (colon == -1) {
name = PersistentService.DIRECTORY + ":" + name;
}
The regression tests ran cleanly with the change, but I'm not yet sure if it is the correct thing to do.
Also note that the problem will only happen when the databases have equal names and are "stored" in the same canonical path (determined by java.io.File).