Bugzilla – Full Text Bug Listing
| Summary: | Test failures when DB4O suite uses client/server DB4OStore | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | [Modeling] EMF | Reporter: | Caspar D. <caspar_d> | ||||||||
| Component: | cdo.core | Assignee: | Caspar D. <caspar_d> | ||||||||
| Status: | CLOSED DUPLICATE | QA Contact: | |||||||||
| Severity: | normal | ||||||||||
| Priority: | P3 | CC: | saulius.tvarijonas, vroldanbet | ||||||||
| Version: | 4.0 | ||||||||||
| Target Milestone: | --- | ||||||||||
| Hardware: | All | ||||||||||
| OS: | All | ||||||||||
| Whiteboard: | |||||||||||
| Attachments: |
|
||||||||||
|
Description
Caspar D.
@Override
protected IStore createStore(String repoName)
{
String path = "/tmp/cdodb.db4o";
// Remove an existing database, unless this is a restart within a test
//
if (!isRestarting())
{
File f = new File(path);
if (f.exists())
{
f.delete();
}
}
return new DB4OStore(path, 50032);
}
The above is a "patch" to be pasted into AllTestsDB4O.java, to instantiate a DB4OStore. Note also this fixes an earlier oversight in this repo config: it didn't take restarts into account. (I already fixed this in my dev branch for the durable locks, see bug 347285.) Furthermore:
MultiValuedOfAttributeTest: 9 failures ("Cannot modify frozen list")
SetFeatureTest: 2 failures (NPE)
CommitInfoTest: 5 failures
ChunkingTest: 4 failures
ExternalReferenceTest: 2 failures
XATransactionTest: 5 failures
Furthermore: BackupTest: 8 failures Bugzilla_258933_Test: 2 failures Bugzilla_259869_Test: 1 failure Bugzilla_329752_Test: 1 failure Hm, that earlier code snippet was a little mindfart.. of course the path would at least have to be qualified with the repoName, otherwise all tests using more than 1 repo are guaranteed to encounter a problem. Running tests again.. pretty slow. Would be nice if we could have optimizing=true for the db4o suite. And the port needs to be randomized:
@Override
protected IStore createStore(String repoName)
{
String path = "/tmp/cdodb_" + repoName + ".db4o";
// Remove an existing database, unless this is a
// restart within a test
//
if (!isRestarting())
{
File f = new File(path);
if (f.exists())
{
f.delete();
}
}
int port = 1024 + random.nextInt(65536 - 1024);
return new DB4OStore(path, port);
}
More trouble: DB4OStore.doDeactivate is flawed. It calls openClient() after calling tearDownObjectServer. Of course, this always fails, but the error is snuffed, and so has gone unnoticed. Yet more trouble: it seems like the DB4O shuts itself down when it encounters an internal error. This means the database gets closed without the test framework knowing about it. And with optimizing==true in the repoconfig, this means the next test encounters a closed database while it expects an open one. And there's a 2nd bug in doDeactivate: it stores an updated instance of the ServerInfo class, which was originally obtained from the db4o database. This is correct, but the mistake is that it's stored using an com.db4o.ObjectContainer that is newly opened during deactivation. As a result, the instance is considered a new object by db4o. After deactivation, there are 2 instances of ServerInfo in the database, which causes an exception (ISE) in initServerInfo on the next startup. And then there's the problem that the DB4O store doesn't recognize the explicit CDORevisionData.NIL object. It actually serializes this object into the store :-) Btw, I'm fixing all of the above in the dev branch that I opened for bug 347285. Created attachment 197062 [details]
Patch
I intend to commit this to my dev branch, but with Eclipse.org's SVN server
broken for now, that's not possible. This fixes all db4o problems not related
to or caused by test optimization.
(Btw, this is a normal unified patch, not an Eclipse patch.)
Created attachment 197065 [details]
Test failures for optimizing==true
Attaching the test failures that occur when the DB4ORepoConfig
is amended to have optimizing==true.
The fact that isOptimizing==true causes failures obviously suggests that there is a problem with re-using the DB4O database accross tests. One thing that looks flawed to me, is that the DB4OStore doesn't seem to have any logic for maintaining the lastObjectID across sessions, or for determining it from the database content. So it looks like the store starts issuing CDOID's from 1 every time it is activated. Another thing I'm not sure is working correctly, is the storage of the rootResourceID. Created attachment 197227 [details]
Patch
(In reply to comment #14) > Created attachment 197227 [details] > Patch That's another normal unified patch. It really needs to go into the dev branch, but I still can't commit. Don't commit to HEAD, there's some debug cruft in there. Committed revision 7926. (dev branch) Subsumed under bug 347285. *** This bug has been marked as a duplicate of bug 347285 *** Caspar, MemDB4OStore was mean to accelerate the test-suite. I originally used to use file-based DB4OStore, but while developing, executing the test-suite took just too much. Do you think its feasible to turn back to its usage by default? Im not sure to understand why there were failures in the MEMBased version and not in the file based version. Is it a DB4O problem, or problems in MEMDB4OStore implementation itself? (In reply to comment #18) Victor, At the risk of sounding a little daft, I don't quite remember. My goal was to fix all the failing tests, and also to rework things so that the DB4O test suite would be able to run with optimizing==true (which means it doesn't tear down and then set up a new repo for every test). I ran into various small bugs on several fronts (some of which I've described earlier in this comment thread), some of which were to do with the MEMDB4OStore, some more with the test config. At some point all tests were passing with the MEMDB4OStore, but not with the regular DB4OStore that I reintroduced, so from then on I made it my goal to make everything pass with the latter, and never looked back to ask myself the question of whether the failures were really due to DB4O-internal differences between the file-based and memory-based implementations. If you want to use the MEMDB4OStore again, I propose that we create a separate config for that, and I recommend you run it with optimizing==true as well. In the case of a memory-based store, the performance advantage is of course not very interesting, but reusing a repo across tests can actually reveal hidden problems. The incorrect storage/retrieval of the ServerInfo object and the last_object_id (which I fixed) was one example of that. Caspar, the sole purpose of MEMDB4OStore was to make the test-suite faster. When developing new functions (DB4OStore doesn't implement some CDO features, like branching), small changes may introduce many errors. Having a mem based implementation makes test-suite way faster to execute (actually, it takes 10 times less than file based, and quite close to H2), making practial the implement-test-fix-test procedure. It was only meant for test performance, not actual production. I agree and I'll create a new bugzilla suggesting to create a separate config for MEMDB4OStore. Thanks for the feedback, and congratulations for such a nice polishing of the DB4OStore implementation (which I actually just implemented in my spare time *for fun*). Are you guys using it in production? Is it providing good performance? I found some points to improve performance, which is still slightly worse than H2 DBStore. Cheers! There you go: https://bugs.eclipse.org/bugs/show_bug.cgi?id=349921 Closing. |