db-derby-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Oystein.Grov...@Sun.COM (Øystein Grøvlen)
Subject Re: [jira] Commented: (DERBY-239) Need a online backup feature that does not block update operations when online backup is in progress.
Date Wed, 04 Jan 2006 09:46:55 GMT


Just a reminder of some things you promised to fix in the previous
review round.


>>>>> "ST" == Suresh Thalamati <suresh.thalamati@gmail.com> writes:


    >> RawStore.java:
    >> * The BACKUP_FILTER now contains so much, that it would be useful
    >> to have a comment that says what is really left to copy.

    ST> I agree,  comments will be helpful  here. I am  not sure BACKUP_FILTER
    ST> will be be needed any more. Currently it just does not filter JAR dir,
    ST> I plan  to make JARS also as  separate copy after the  data segment as
    ST> discussed in my previous e-mail.


    >> * I think it would be helpful with a comment that explained why
    >> disableLogArchiveMode() was made synchronized.

    ST> Yes, I definitely need to add comments about this synchronization.
    ST> This was part of my  attempt to prevent parallel backup actions, which
    ST> I did not complete as part of this patch.


    >> RAFContainer.java:


    >> * privBackupContainer():  - Setting 'done=true' at  the start of
    >> the method is a bit

    >> confusing.  I would think another name for this variable
    >> would be better.

    ST> I  will change it  to 'backupComplete  = false'  and reverse  the exit
    ST> logic, hopefully that will make it more readable.

    >> - If an exception is thrown while holding a latch, do one
    >> not need to relase the latch?

    ST> Yes. Latch  has to be released. I  will make sure no  latches are held
    ST> when an error occurs

    >> * writeToBackup(): - copies a lot of code from writePage().  One
    >> should

    >> consider factoring out common code.

    ST> I will move the common code into a separate routine.


    >> LogToFile.java:


    >> * The javadoc for startLogBackup() says that 'log files are copied
    >> after all the data files are backed up, but at the end of the
    >> method log files are copied.

    >> ReadOnly.java/InputStreamContainer.java:  * I  do not  think javadoc
    >> should just be a copy of the

    >> interface/superclass, but say something about what is particular
    >> to this implementation.  In this case, the Javadoc should say
    >> that nothing is done.

    >> typos:

    >> * several occurences of 'backedup'
    >> * LogToFile.java: 'eventhough'
    >> * RAFContainer.java: 'pahe cache', 'conatiner'
    >> * 'Standard Cloudscape error policy'.  Should this be changed to
    >> Derby?

    ST> I will correct the comments.

    >> Pet peeves:

    >> * Should have Javadoc on all methods, parameters and return
    >> values.
    >> * Lines longer than 80 chars.

    ST> Sound like a good ones to follow.  I will add the java doc and fix any
    ST> long lines.

    ST> Thanks
    ST> -suresh

View raw message