Hi Sunitha,
Good comments. I am surprised to find that this patch doing more than what I imagined it will do, your changes
seems to allow switching between durable test mode(No syncs) and the default durability level (all sync). I don't get why
these mode switch functionality is required, if this mode is just meant to run the tests faster in non-durable mode! . Not sure what
kind of new problem scenarios this mode switching will introduce.
But for some reason if some one wants, I am not sure your fix will actually make a non-durable database
to durable/consistent one correctly. What I learn from your changes is that data is not synced to disk on checkpoints,
if you don't do that you can not assume database is is in consistent state if recovery went through fine. Let's say:
1) user started the database with derby.system.testMode=true 2) did some insert/update/deletes ... 3) crashed immediately after the checpoint record got on to disk... 4) Now user switches derby.system.testMode=false and boot the database.
Recovery will work fine because there is no log to replay. So you unmark the
Test mode flag in the control file and declare that this database is in durable/consistent mode.
But that is not the case because only some of the data pages might have reached to the
disk before the crash, Which will actually leave the database inconsistent state.
(This is different from losing some transactions).
Some general comments on the code:
1) Why don't we just introduce a new flag private static final byte TESTMODE_NO_SYNC_TRUE = 0x2 and
write as part of the "flags" byte which is currently used to indicate the IS_BETA_FLAG instead of using
another spare byte.
2) in few places indentation is not matching the rest of the code( that's how my emacs shows it):
a)
// if system is running with derby.system.testMode enabled
// log records and log file are not synced, so
// set flag to disable log syncs
b)
{
// print message stating that the database was
// previously running with derby.system.testMode=true
// mode
c) if( wasDBInTestModeNoSync && !logNotSynced )
{
// write to log control file and the mirror file that the testmode
// is not used now
d) if(wasDBInTestModeNoSync)
{
// print message stating that the database was
// previously running with derby.system.testMode=true
......etc.
3) I think the logical place for the method private boolean updateTestModeFlagInCtrlFile(byte value)
is next to writeControlFile(...) .
4) protected logErrMsgForTestModeNoSync() may be marked as a private.
Thanks -suresht
Sunitha Kambhampati wrote:
A little background: Sometime earlier on the list, Dan posted a fix to make derby go faster with relaxed durability with some flags. The post is at http://article.gmane.org/gmane.comp.apache.db.derby.user/681/match=relaxed+durability
This mode is very useful for unit testing or at development time when recoverability is not required.
Basically in this mode, syncs are disabled for logs, data writes at checkpoint, page allocation when file is grown; - what this means is that data is not flushed all the way to the disk and in most cases i/o cost is not involved. Note, code already exists in Derby to do this.
So for Derby 218, This patch addresses the following requirements: 1) Have a single property to enable this relaxed durability mode, so it is easy for users to enable it.
2) Print message to derby.log that this mode is enabled
3) A way to report boot errors that may be because of this option being enabled. What this maps to is - have a marker to recognize that database was booted in this mode, and then on subsequent boot; if errors happen during recovery - report a message that it could have happened because of this mode being set.
Ch
............
