Derby
  1. Derby
  2. DERBY-5232

Put a stern README file in log and seg0 directories to warn users of corrpution they will cause if they touch files there

    Details

    • Type: Improvement Improvement
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 10.8.1.2
    • Fix Version/s: 10.10.1.1
    • Component/s: Store
    • Labels:
      None
    • Issue & fix info:
      High Value Fix, Release Note Needed

      Description

      Users often on bad advice or desperation touch or delete files in the log or seg0 directories (mostly log).

      I think it would be good for new databases and on upgrade that a file be created in these directories with a name like:
      TOUCHING_FILES_HERE_WILL_CORRUPT_DB_README.txt

      or some such to warn of the perils of doing this and the corrupting effects and how it can eliminate any possibility of salvage. It should also encourage users to make a zip backup of the database directory with no jvm currently accessing it before trying to do anything with the database if it appears to be already corrupt. Also point to backup/restore documentation and encourage restore of a good backup into an empty directory if the database is corrupt.

      I'm not sure if it would help but it couldn't hurt.

      1. DERBY5232_patch1_diff.txt
        5 kB
        Mamta A. Satoor
      2. DERBY5232_patch1_stat.txt
        0.3 kB
        Mamta A. Satoor
      3. DERBY5232_patch2_diff.txt
        13 kB
        Mamta A. Satoor
      4. DERBY5232_patch2_stat.txt
        0.6 kB
        Mamta A. Satoor
      5. DERBY5232_patch3_diff.txt
        25 kB
        Mamta A. Satoor
      6. DERBY5232_patch3_stat.txt
        1 kB
        Mamta A. Satoor
      7. DERBY5232_patch4_diff.txt
        15 kB
        Mamta A. Satoor
      8. DERBY5232_patch4_stat.txt
        0.7 kB
        Mamta A. Satoor
      9. DERBY5232_patch5_diff.txt
        3 kB
        Mamta A. Satoor
      10. DERBY5232_patch5_stat.txt
        0.2 kB
        Mamta A. Satoor
      11. DERBY5232_patch6_diff.txt
        5 kB
        Mamta A. Satoor
      12. DERBY5232_patch6_stat.txt
        0.2 kB
        Mamta A. Satoor
      13. DERBY5232_patch7_diff.txt
        9 kB
        Mamta A. Satoor
      14. DERBY5232_patch7_stat.txt
        0.3 kB
        Mamta A. Satoor
      15. DERBY5232_test_patch1_diff.txt
        6 kB
        Mamta A. Satoor
      16. DERBY5232_test_patch1_stat.txt
        0.2 kB
        Mamta A. Satoor
      17. DERBY5232_test_patch2_diff.txt
        8 kB
        Mamta A. Satoor
      18. DERBY5232_test_patch2_stat.txt
        0.4 kB
        Mamta A. Satoor
      19. DERBY-5232-kim.diff
        3 kB
        Kim Haase
      20. releaseNote.html
        4 kB
        Mamta A. Satoor
      21. releaseNote.html
        4 kB
        Mamta A. Satoor
      22. releaseNote.html
        4 kB
        Mamta A. Satoor

        Issue Links

          Activity

          Gavin made changes -
          Workflow jira [ 12613440 ] Default workflow, editable Closed status [ 12802212 ]
          Mamta A. Satoor made changes -
          Status Open [ 1 ] Resolved [ 5 ]
          Resolution Fixed [ 1 ]
          Hide
          Mamta A. Satoor added a comment -

          Thanks for checking the release notes, Knut. I will go ahead and close this jira.

          Show
          Mamta A. Satoor added a comment - Thanks for checking the release notes, Knut. I will go ahead and close this jira.
          Hide
          Knut Anders Hatlen added a comment -

          Thanks, Mamta. The latest release note looks fine to me.

          Show
          Knut Anders Hatlen added a comment - Thanks, Mamta. The latest release note looks fine to me.
          Mamta A. Satoor made changes -
          Attachment releaseNote.html [ 12560722 ]
          Hide
          Mamta A. Satoor added a comment -

          Attaching a new release note based on Knut's suggestion. This release note should cover DERBY-5232 and DERBY-5996. Thanks

          Show
          Mamta A. Satoor added a comment - Attaching a new release note based on Knut's suggestion. This release note should cover DERBY-5232 and DERBY-5996 . Thanks
          Hide
          Mamta A. Satoor added a comment -

          Thanks for your suggestions, Knut. I will incorporate them and attach a new release note.

          Show
          Mamta A. Satoor added a comment - Thanks for your suggestions, Knut. I will incorporate them and attach a new release note.
          Hide
          Knut Anders Hatlen added a comment -

          Thanks for writing the release note, Mamta. Some suggestions:

          The summary is supposed to be a one line, one sentence summary. What about "Derby will create readme files in the database directories"?

          I think the symptoms section should describe the symptoms seen when upgrading to the new Derby release, not the symptoms seen when ignoring the advice in the readme files. For example: "A database created with Derby 10.10, or upgraded to 10.10, will contain readme files named README_DO_NOT_TOUCH_FILES.txt in the top-level database directory and in the subdirectories seg0 and log."

          Show
          Knut Anders Hatlen added a comment - Thanks for writing the release note, Mamta. Some suggestions: The summary is supposed to be a one line, one sentence summary. What about "Derby will create readme files in the database directories"? I think the symptoms section should describe the symptoms seen when upgrading to the new Derby release, not the symptoms seen when ignoring the advice in the readme files. For example: "A database created with Derby 10.10, or upgraded to 10.10, will contain readme files named README_DO_NOT_TOUCH_FILES.txt in the top-level database directory and in the subdirectories seg0 and log."
          Mamta A. Satoor made changes -
          Attachment releaseNote.html [ 12560201 ]
          Hide
          Mamta A. Satoor added a comment -

          Attaching release note relevant to this jira

          Show
          Mamta A. Satoor added a comment - Attaching release note relevant to this jira
          Hide
          Mamta A. Satoor added a comment -

          Knut, you are right. I have multiple trunk clients on my machine and accidentally copied the wrong release notes. I will copy the correct one soon.Thanks

          Show
          Mamta A. Satoor added a comment - Knut, you are right. I have multiple trunk clients on my machine and accidentally copied the wrong release notes. I will copy the correct one soon.Thanks
          Hide
          Knut Anders Hatlen added a comment -

          Did you attach the right release note? The one I saw when I clicked on the attachment, talked about trigger data corruption, which sounds like a different issue.

          Show
          Knut Anders Hatlen added a comment - Did you attach the right release note? The one I saw when I clicked on the attachment, talked about trigger data corruption, which sounds like a different issue.
          Mamta A. Satoor made changes -
          Attachment releaseNote.html [ 12560141 ]
          Hide
          Mamta A. Satoor added a comment -

          Attaching release notes for the jira. Please let me know if there is any feedback on the wordings of the release note.

          Show
          Mamta A. Satoor added a comment - Attaching release notes for the jira. Please let me know if there is any feedback on the wordings of the release note.
          Mamta A. Satoor made changes -
          Link This issue is related to DERBY-5996 [ DERBY-5996 ]
          Mamta A. Satoor made changes -
          Link This issue is related to DERBY-5995 [ DERBY-5995 ]
          Hide
          Mamta A. Satoor added a comment -

          Committed the changes suggested by Kim with revision 1410465.

          Show
          Mamta A. Satoor added a comment - Committed the changes suggested by Kim with revision 1410465.
          Hide
          Mamta A. Satoor added a comment -

          Hi Kim, thanks for taking the time to provide the feedback. I have changed the wording of the readme files in messages.xml as per your suggestion. I would like us to stay consistent with the error messages and use the term NON-RECOVERABLE in the readme files. Also, to avoid the confusion and for the text to look consistent, I have changed "Do not touch files in this directory!" to all upper case. I will go ahead and commit these changes shortly.

          Show
          Mamta A. Satoor added a comment - Hi Kim, thanks for taking the time to provide the feedback. I have changed the wording of the readme files in messages.xml as per your suggestion. I would like us to stay consistent with the error messages and use the term NON-RECOVERABLE in the readme files. Also, to avoid the confusion and for the text to look consistent, I have changed "Do not touch files in this directory!" to all upper case. I will go ahead and commit these changes shortly.
          Kim Haase made changes -
          Attachment DERBY-5232-kim.diff [ 12553664 ]
          Hide
          Kim Haase added a comment -

          Happy to help with this. I agree completely with Kathey's suggestion about the file names – the separate words are indeed preferable to the contraction.

          I am attaching a suggested patch for messages.xml, DERBY-5232-kim.diff, with some wording fixes. (I hope it merges all right with Mamta's latest.) I do have a couple of questions –

          For consistency, I used the term NON-RECOVERABLE, since that's what we use in our SQLSTate error messages. However, the Oracle terminology guide for my organization says to use NONRECOVERABLE (no hyphen), while the official Oracle style guide says to use IRRECOVERABLE or NOT RECOVERABLE. What does the IBM style guide say? Or should we just worry about consistency with the error messages?

          I notice a comment saying, "Translators: Please translate the ALL CAPS words." However, there's a mixed case sentence at the top of each saying, "Do not touch files in this directory!" I assume we want this to be translated too – is it understood that they always translate mixed-case text but not all-caps text? Sorry to be ignorant about our localization practices.

          Show
          Kim Haase added a comment - Happy to help with this. I agree completely with Kathey's suggestion about the file names – the separate words are indeed preferable to the contraction. I am attaching a suggested patch for messages.xml, DERBY-5232 -kim.diff, with some wording fixes. (I hope it merges all right with Mamta's latest.) I do have a couple of questions – For consistency, I used the term NON-RECOVERABLE, since that's what we use in our SQLSTate error messages. However, the Oracle terminology guide for my organization says to use NONRECOVERABLE (no hyphen), while the official Oracle style guide says to use IRRECOVERABLE or NOT RECOVERABLE. What does the IBM style guide say? Or should we just worry about consistency with the error messages? I notice a comment saying, "Translators: Please translate the ALL CAPS words." However, there's a mixed case sentence at the top of each saying, "Do not touch files in this directory!" I assume we want this to be translated too – is it understood that they always translate mixed-case text but not all-caps text? Sorry to be ignorant about our localization practices.
          Hide
          Mamta A. Satoor added a comment -

          Hi Kathey, thanks for your suggestion. I have renamed the file to be more user friendly especially to non-English readers. The change went in as revision 1409654

          Yes, it will be great if Kim had any comments on the text or the name of of the readme files.

          Show
          Mamta A. Satoor added a comment - Hi Kathey, thanks for your suggestion. I have renamed the file to be more user friendly especially to non-English readers. The change went in as revision 1409654 Yes, it will be great if Kim had any comments on the text or the name of of the readme files.
          Hide
          Kathey Marsden added a comment -

          Hi Mamta,

          I have not had a chance to look closely and won't until I get 10.8.3 out. I have some cosmetic comments but can file a new issue once I get them together. I can just file a new issue if preferrable. The most significant is:
          I would like to change
          README_DONT_TOUCH_FILES.txt to
          README_DO_NOT_TOUCH_FILES.txt

          The contraction without an appostrophy is bothersome and since I am guessing the file name is not localized, having separate words will be more clear to non-English readers.

          I think it would be good for Kim to review the text. I will sign up to buddy test this feature for 10.10. Thank you so much for putting this warning to users in Derby!

          Show
          Kathey Marsden added a comment - Hi Mamta, I have not had a chance to look closely and won't until I get 10.8.3 out. I have some cosmetic comments but can file a new issue once I get them together. I can just file a new issue if preferrable. The most significant is: I would like to change README_DONT_TOUCH_FILES.txt to README_DO_NOT_TOUCH_FILES.txt The contraction without an appostrophy is bothersome and since I am guessing the file name is not localized, having separate words will be more clear to non-English readers. I think it would be good for Kim to review the text. I will sign up to buddy test this feature for 10.10. Thank you so much for putting this warning to users in Derby!
          Hide
          Mamta A. Satoor added a comment -

          I have created DERBY-5995 for adding a test case when log directory has been changed with the jbdc url attribute logDevice. I should have a patch for DERBY-5995 by tomorrow. I will go ahead and close DERBY-5232 if they are no further comments on the work that has gone in so far.

          Show
          Mamta A. Satoor added a comment - I have created DERBY-5995 for adding a test case when log directory has been changed with the jbdc url attribute logDevice. I should have a patch for DERBY-5995 by tomorrow. I will go ahead and close DERBY-5232 if they are no further comments on the work that has gone in so far.
          Mamta A. Satoor made changes -
          Link This issue relates to DERBY-5995 [ DERBY-5995 ]
          Hide
          Mamta A. Satoor added a comment -

          Hi Knut, decided to get rid of the length check on the file, it is probably plenty to check that the file exists. Added the assert on File exists check though. Made these changes with revision 1409254.

          Show
          Mamta A. Satoor added a comment - Hi Knut, decided to get rid of the length check on the file, it is probably plenty to check that the file exists. Added the assert on File exists check though. Made these changes with revision 1409254.
          Hide
          Knut Anders Hatlen added a comment -

          Some comments to the 1409100 commit:

          The new run() method is declared as throws IOException, but the catch block unconditionally casts the exception to FileNotFoundException. I'm worried that this might lead to a ClassNotFoundException that hides the original IOException.

          The lookForReadmeFile() method doesn't check the return value of the calls. It should assert return value true from the exists() call and false from isFileEmpty(). What about just relying on the existing methods in PrivilegedFileOpsForTests and skip the new isFileEmpty() method? lookForReadmeFile() could simply do

          assertTrue(readmeFile + " doesn't exist", PrivilegedFileOpsForTests.exists(readmeFile));
          assertNotSame(readmeFile + " is empty", 0, PrivilegedFileOpsForTests.length(readmeFile));

          Show
          Knut Anders Hatlen added a comment - Some comments to the 1409100 commit: The new run() method is declared as throws IOException, but the catch block unconditionally casts the exception to FileNotFoundException. I'm worried that this might lead to a ClassNotFoundException that hides the original IOException. The lookForReadmeFile() method doesn't check the return value of the calls. It should assert return value true from the exists() call and false from isFileEmpty(). What about just relying on the existing methods in PrivilegedFileOpsForTests and skip the new isFileEmpty() method? lookForReadmeFile() could simply do assertTrue(readmeFile + " doesn't exist", PrivilegedFileOpsForTests.exists(readmeFile)); assertNotSame(readmeFile + " is empty", 0, PrivilegedFileOpsForTests.length(readmeFile));
          Hide
          Mamta A. Satoor added a comment -

          I wanted to go ahead and add the test case fo the existence of the 3 readme files for a simple default database creation. That commit went as revision 1409100 with commit comments
          ************************
          DERBY-5232 (Put a stern README file in log and seg0 directories to warn users of corrpution they will cause if they touch files there)

          Adding a test case for default database creation with the checks for the existence of the 3 readme files. These readmes warn users against editing/deleting any of the files in the database directories. The location of the readme files are
          1)at the db level directory,
          2)in seg0 directory and
          3)in the log directocy.
          All the three readme files are named README_DONT_TOUCH_FILES.txt
          ************************

          Show
          Mamta A. Satoor added a comment - I wanted to go ahead and add the test case fo the existence of the 3 readme files for a simple default database creation. That commit went as revision 1409100 with commit comments ************************ DERBY-5232 (Put a stern README file in log and seg0 directories to warn users of corrpution they will cause if they touch files there) Adding a test case for default database creation with the checks for the existence of the 3 readme files. These readmes warn users against editing/deleting any of the files in the database directories. The location of the readme files are 1)at the db level directory, 2)in seg0 directory and 3)in the log directocy. All the three readme files are named README_DONT_TOUCH_FILES.txt ************************
          Hide
          Mamta A. Satoor added a comment -

          I think the issue with my earlier patch might be the order in which TestConfigruations are getting created and fixing that probably will make the patch do the right thing. I am looking into it.

          Show
          Mamta A. Satoor added a comment - I think the issue with my earlier patch might be the order in which TestConfigruations are getting created and fixing that probably will make the patch do the right thing. I am looking into it.
          Mamta A. Satoor made changes -
          Attachment DERBY5232_test_patch2_diff.txt [ 12553254 ]
          Attachment DERBY5232_test_patch2_stat.txt [ 12553255 ]
          Hide
          Mamta A. Satoor added a comment -

          I have changed the earlier test patch to use getDefaultDatabaseName() to find the database path to look for the readme files inside the database directory and inside the seg0 directory. Also, I have added one helper method in PrivilegedFileOpsForTests class and used one of the existing methods in that class to find the readme files. Since a run of network server after embedded does not really recreate the database, the test now only runs in embedded mode and ensures that 3 readme files get created correctly.

          The issue that's left now is deleting the log files from the new log location set by the logDevice attribute. I thought I would be able to do that in the DropDatabaseSetup.removeDatabase() method with the following code
          String logDevice = config.getConnectionAttributes().getProperty("logDevice");
          if (logDevice != null)

          { removeDirectory(logDevice); }

          But "logDevice" property comes back as null as if that property was never set. Through debugging, I am finding that the reason behind this is TestConfiguration.getCurrent() by this point has lost the TestConfiguration used to do the testing and hence we have lost the logDevice property from the available properties. Maybe that is the right thing to happen but I thought the tearDown of DropDatabase.tearDown() should get called before ChangeConfigurationSetup.tearDown() which is where the TestConfiguration used by the test gets overwritten. The code for ChangeConfigurationSetup.tearDown() is as follows for reference.
          protected final void tearDown()

          { TestConfiguration.setCurrent(old); }

          Maybe I am trying to solve it in the wrong place but I don't understand why the current configuration gets overwritten before all the decorator have a chance to do their on tear down. Would appreciate someone explaining the current behavior.

          In the mean time, I will investigate Knut's suggestion that may be the log directory deletion should happen in the Decorator returned by Decorator.logDeviceAttributeDatabase(). I will try to investigate that.

          Show
          Mamta A. Satoor added a comment - I have changed the earlier test patch to use getDefaultDatabaseName() to find the database path to look for the readme files inside the database directory and inside the seg0 directory. Also, I have added one helper method in PrivilegedFileOpsForTests class and used one of the existing methods in that class to find the readme files. Since a run of network server after embedded does not really recreate the database, the test now only runs in embedded mode and ensures that 3 readme files get created correctly. The issue that's left now is deleting the log files from the new log location set by the logDevice attribute. I thought I would be able to do that in the DropDatabaseSetup.removeDatabase() method with the following code String logDevice = config.getConnectionAttributes().getProperty("logDevice"); if (logDevice != null) { removeDirectory(logDevice); } But "logDevice" property comes back as null as if that property was never set. Through debugging, I am finding that the reason behind this is TestConfiguration.getCurrent() by this point has lost the TestConfiguration used to do the testing and hence we have lost the logDevice property from the available properties. Maybe that is the right thing to happen but I thought the tearDown of DropDatabase.tearDown() should get called before ChangeConfigurationSetup.tearDown() which is where the TestConfiguration used by the test gets overwritten. The code for ChangeConfigurationSetup.tearDown() is as follows for reference. protected final void tearDown() { TestConfiguration.setCurrent(old); } Maybe I am trying to solve it in the wrong place but I don't understand why the current configuration gets overwritten before all the decorator have a chance to do their on tear down. Would appreciate someone explaining the current behavior. In the mean time, I will investigate Knut's suggestion that may be the log directory deletion should happen in the Decorator returned by Decorator.logDeviceAttributeDatabase(). I will try to investigate that.
          Hide
          Knut Anders Hatlen added a comment -

          I think the database will be created in a subdirectory under system/singleUse, and the name will vary depending on the number of single-use databases created earlier in the test run. So to find the database directory, you'd need something like this:

          TestConfiguration config = TestConfiguration.getCurrent();
          File readMeFile = new File(
          config.getDatabasePath(config.getDefaultDatabaseName()),
          DB_README_FILE_NAME);

          The new assert methods could be simplified by using helper methods from the PrivilegedFileOpsForTests class (would avoid the need for doPrivileged() blocks).

          The test runs in both embedded and client/server mode. However, it only creates the database once, so starting the server and re-checking that the files still exist doesn't add much value. I'd suggest that you replace the defaultSuite() decorator with embeddedSuite() in the suite() method so that it only runs once. (If you make this change, you may also have to add a getConnection() call to the test case to get the database created. defaultSuite() adds some decorators that will call getConnection() before running the test, but I don't think embeddedSuite() does that.)

          The test should also delete the logDevice directory when it's done, as that won't be done automatically by DropDatabaseSetup. It should probably be done by the decorator returned by Decorator.logDeviceAttributeDatabase() and not by the test itself.

          Some nits:

          • The new file uses a mix of tabs and spaces for indentation
          • I'd prefer braces in the if statement in Decorator.logDeviceAttributeDatabase() (my IDE shows a warning because of the missing braces). Or perhaps just remove the null check, as those tests that want to use the default logDevice, should not use this decorator.
          Show
          Knut Anders Hatlen added a comment - I think the database will be created in a subdirectory under system/singleUse, and the name will vary depending on the number of single-use databases created earlier in the test run. So to find the database directory, you'd need something like this: TestConfiguration config = TestConfiguration.getCurrent(); File readMeFile = new File( config.getDatabasePath(config.getDefaultDatabaseName()), DB_README_FILE_NAME); The new assert methods could be simplified by using helper methods from the PrivilegedFileOpsForTests class (would avoid the need for doPrivileged() blocks). The test runs in both embedded and client/server mode. However, it only creates the database once, so starting the server and re-checking that the files still exist doesn't add much value. I'd suggest that you replace the defaultSuite() decorator with embeddedSuite() in the suite() method so that it only runs once. (If you make this change, you may also have to add a getConnection() call to the test case to get the database created. defaultSuite() adds some decorators that will call getConnection() before running the test, but I don't think embeddedSuite() does that.) The test should also delete the logDevice directory when it's done, as that won't be done automatically by DropDatabaseSetup. It should probably be done by the decorator returned by Decorator.logDeviceAttributeDatabase() and not by the test itself. Some nits: The new file uses a mix of tabs and spaces for indentation I'd prefer braces in the if statement in Decorator.logDeviceAttributeDatabase() (my IDE shows a warning because of the missing braces). Or perhaps just remove the null check, as those tests that want to use the default logDevice, should not use this decorator.
          Mamta A. Satoor made changes -
          Attachment DERBY5232_test_patch1_diff.txt [ 12552743 ]
          Attachment DERBY5232_test_patch1_stat.txt [ 12552744 ]
          Hide
          Mamta A. Satoor added a comment -

          I am attaching a patch for adding a junit test to check if the readme file gets created in the log directory when the log is directed to point to a different location than the default database directory. This change of log directory is done using the logDevice jdbc url attribute. eg
          java -Dij.exceptionTrace=true org.apache.derby.tools.ij
          connect 'jdbc:derby:db1;create=true;logDevice=../dellater';
          exit;
          The manual testing of the logDevice shows that readme file has been created inside the non-default log directory.

          The support for logDevice doesn't yet exist in our junit test framework. I have added changes in this patch to provide logDevice support inside junit.

          But with my test, what I am finding is that even though the log got moved to the new location, the actual database files are not under the singleUse database directory. Not sure if my changes for logDevice support is causing this problem. Would appreciate any feedback from someone familiar with the junit framework in this area to see what might be wrong. In order to run the new added junit test, one can use following command
          time java -Dderby.tests.trace=true junit.textui.TestRunner org.apache.derbyTesting.functionTests.tests.engine.ReadMeFilesTest > runallOneTest.out 2>&1
          The test above gives following results

          $ time java -Dderby.tests.trace=true junit.textui.TestRunner org.apache.derbyTesting.functionTests.tests.engine.ReadMeFilesTest
          .
          (emb)engine.ReadMeFilesTest.testReadMeFilesExist C:\p4clients\svnmain\client3\trunk\systest\out142\system\singleUse
          used 820 ms F.
          (net)engine.ReadMeFilesTest.testReadMeFilesExist C:\p4clients\svnmain\client3\trunk\systest\out142\system\singleUse
          used 230 ms F
          Time: 5.096
          There were 2 failures:
          1) testReadMeFilesExist(org.apache.derbyTesting.functionTests.tests.engine.ReadMeFilesTest)junit.framework.AssertionFailedError: assertIsExisting failed: C:\p4clients\svnmain\client3\trunk\systest\out142\system\singleUse\README_DONT_TOUCH_FILES.txt
          at org.apache.derbyTesting.functionTests.tests.engine.ReadMeFilesTest$1.run(ReadMeFilesTest.java:121)
          at java.security.AccessController.doPrivileged(AccessController.java:251)
          at org.apache.derbyTesting.functionTests.tests.engine.ReadMeFilesTest.assertIsExisting(ReadMeFilesTest.java:119)
          at org.apache.derbyTesting.functionTests.tests.engine.ReadMeFilesTest.testReadMeFilesExist(ReadMeFilesTest.java:101)
          at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
          at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:60)
          at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:37)
          at org.apache.derbyTesting.junit.BaseTestCase.runBare(BaseTestCase.java:117)
          at org.apache.derbyTesting.junit.BaseJDBCTestCase.runBareOverridable(BaseJDBCTestCase.java:424)
          at org.apache.derbyTesting.junit.BaseJDBCTestCase.runBare(BaseJDBCTestCase.java:441)
          at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24)
          at junit.extensions.TestSetup$1.protect(TestSetup.java:21)
          at junit.extensions.TestSetup.run(TestSetup.java:25)
          at org.apache.derbyTesting.junit.BaseTestSetup.run(BaseTestSetup.java:57)
          at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24)
          at junit.extensions.TestSetup$1.protect(TestSetup.java:21)
          at junit.extensions.TestSetup.run(TestSetup.java:25)
          at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24)
          at junit.extensions.TestSetup$1.protect(TestSetup.java:21)
          at junit.extensions.TestSetup.run(TestSetup.java:25)
          at org.apache.derbyTesting.junit.BaseTestSetup.run(BaseTestSetup.java:57)
          at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24)
          at junit.extensions.TestSetup$1.protect(TestSetup.java:21)
          at junit.extensions.TestSetup.run(TestSetup.java:25)
          2) testReadMeFilesExist(org.apache.derbyTesting.functionTests.tests.engine.ReadMeFilesTest)junit.framework.AssertionFailedError: assertIsExisting failed: C:\p4clients\svnmain\client3\trunk\systest\out142\system\singleUse\README_DONT_TOUCH_FILES.txt
          at org.apache.derbyTesting.functionTests.tests.engine.ReadMeFilesTest$1.run(ReadMeFilesTest.java:121)
          at java.security.AccessController.doPrivileged(AccessController.java:251)
          at org.apache.derbyTesting.functionTests.tests.engine.ReadMeFilesTest.assertIsExisting(ReadMeFilesTest.java:119)
          at org.apache.derbyTesting.functionTests.tests.engine.ReadMeFilesTest.testReadMeFilesExist(ReadMeFilesTest.java:101)
          at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
          at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:60)
          at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:37)
          at org.apache.derbyTesting.junit.BaseTestCase.runBare(BaseTestCase.java:117)
          at org.apache.derbyTesting.junit.BaseJDBCTestCase.runBareOverridable(BaseJDBCTestCase.java:424)
          at org.apache.derbyTesting.junit.BaseJDBCTestCase.runBare(BaseJDBCTestCase.java:441)
          at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24)
          at junit.extensions.TestSetup$1.protect(TestSetup.java:21)
          at junit.extensions.TestSetup.run(TestSetup.java:25)
          at org.apache.derbyTesting.junit.BaseTestSetup.run(BaseTestSetup.java:57)
          at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24)
          at junit.extensions.TestSetup$1.protect(TestSetup.java:21)
          at junit.extensions.TestSetup.run(TestSetup.java:25)
          at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24)
          at junit.extensions.TestSetup$1.protect(TestSetup.java:21)
          at junit.extensions.TestSetup.run(TestSetup.java:25)
          at org.apache.derbyTesting.junit.BaseTestSetup.run(BaseTestSetup.java:57)
          at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24)
          at junit.extensions.TestSetup$1.protect(TestSetup.java:21)
          at junit.extensions.TestSetup.run(TestSetup.java:25)
          at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24)
          at junit.extensions.TestSetup$1.protect(TestSetup.java:21)
          at junit.extensions.TestSetup.run(TestSetup.java:25)
          at org.apache.derbyTesting.junit.BaseTestSetup.run(BaseTestSetup.java:57)
          at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24)
          at junit.extensions.TestSetup$1.protect(TestSetup.java:21)
          at junit.extensions.TestSetup.run(TestSetup.java:25)

          FAILURES!!!
          Tests run: 2, Failures: 2, Errors: 0

          real 0m6.508s
          user 0m0.000s
          sys 0m0.015s

          The existing test for logDevice jdbc url attribute are still in the old test harness
          java\testing\org\apache\derbyTesting\functionTests\tests\store\LogDeviceTest.java
          I can always go and do my testing for existence of readme files in that old harness test but it will be good to add the support for this attribute in junit framework.

          Show
          Mamta A. Satoor added a comment - I am attaching a patch for adding a junit test to check if the readme file gets created in the log directory when the log is directed to point to a different location than the default database directory. This change of log directory is done using the logDevice jdbc url attribute. eg java -Dij.exceptionTrace=true org.apache.derby.tools.ij connect 'jdbc:derby:db1;create=true;logDevice=../dellater'; exit; The manual testing of the logDevice shows that readme file has been created inside the non-default log directory. The support for logDevice doesn't yet exist in our junit test framework. I have added changes in this patch to provide logDevice support inside junit. But with my test, what I am finding is that even though the log got moved to the new location, the actual database files are not under the singleUse database directory. Not sure if my changes for logDevice support is causing this problem. Would appreciate any feedback from someone familiar with the junit framework in this area to see what might be wrong. In order to run the new added junit test, one can use following command time java -Dderby.tests.trace=true junit.textui.TestRunner org.apache.derbyTesting.functionTests.tests.engine.ReadMeFilesTest > runallOneTest.out 2>&1 The test above gives following results $ time java -Dderby.tests.trace=true junit.textui.TestRunner org.apache.derbyTesting.functionTests.tests.engine.ReadMeFilesTest . (emb)engine.ReadMeFilesTest.testReadMeFilesExist C:\p4clients\svnmain\client3\trunk\systest\out142\system\singleUse used 820 ms F. (net)engine.ReadMeFilesTest.testReadMeFilesExist C:\p4clients\svnmain\client3\trunk\systest\out142\system\singleUse used 230 ms F Time: 5.096 There were 2 failures: 1) testReadMeFilesExist(org.apache.derbyTesting.functionTests.tests.engine.ReadMeFilesTest)junit.framework.AssertionFailedError: assertIsExisting failed: C:\p4clients\svnmain\client3\trunk\systest\out142\system\singleUse\README_DONT_TOUCH_FILES.txt at org.apache.derbyTesting.functionTests.tests.engine.ReadMeFilesTest$1.run(ReadMeFilesTest.java:121) at java.security.AccessController.doPrivileged(AccessController.java:251) at org.apache.derbyTesting.functionTests.tests.engine.ReadMeFilesTest.assertIsExisting(ReadMeFilesTest.java:119) at org.apache.derbyTesting.functionTests.tests.engine.ReadMeFilesTest.testReadMeFilesExist(ReadMeFilesTest.java:101) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:60) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:37) at org.apache.derbyTesting.junit.BaseTestCase.runBare(BaseTestCase.java:117) at org.apache.derbyTesting.junit.BaseJDBCTestCase.runBareOverridable(BaseJDBCTestCase.java:424) at org.apache.derbyTesting.junit.BaseJDBCTestCase.runBare(BaseJDBCTestCase.java:441) at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24) at junit.extensions.TestSetup$1.protect(TestSetup.java:21) at junit.extensions.TestSetup.run(TestSetup.java:25) at org.apache.derbyTesting.junit.BaseTestSetup.run(BaseTestSetup.java:57) at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24) at junit.extensions.TestSetup$1.protect(TestSetup.java:21) at junit.extensions.TestSetup.run(TestSetup.java:25) at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24) at junit.extensions.TestSetup$1.protect(TestSetup.java:21) at junit.extensions.TestSetup.run(TestSetup.java:25) at org.apache.derbyTesting.junit.BaseTestSetup.run(BaseTestSetup.java:57) at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24) at junit.extensions.TestSetup$1.protect(TestSetup.java:21) at junit.extensions.TestSetup.run(TestSetup.java:25) 2) testReadMeFilesExist(org.apache.derbyTesting.functionTests.tests.engine.ReadMeFilesTest)junit.framework.AssertionFailedError: assertIsExisting failed: C:\p4clients\svnmain\client3\trunk\systest\out142\system\singleUse\README_DONT_TOUCH_FILES.txt at org.apache.derbyTesting.functionTests.tests.engine.ReadMeFilesTest$1.run(ReadMeFilesTest.java:121) at java.security.AccessController.doPrivileged(AccessController.java:251) at org.apache.derbyTesting.functionTests.tests.engine.ReadMeFilesTest.assertIsExisting(ReadMeFilesTest.java:119) at org.apache.derbyTesting.functionTests.tests.engine.ReadMeFilesTest.testReadMeFilesExist(ReadMeFilesTest.java:101) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:60) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:37) at org.apache.derbyTesting.junit.BaseTestCase.runBare(BaseTestCase.java:117) at org.apache.derbyTesting.junit.BaseJDBCTestCase.runBareOverridable(BaseJDBCTestCase.java:424) at org.apache.derbyTesting.junit.BaseJDBCTestCase.runBare(BaseJDBCTestCase.java:441) at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24) at junit.extensions.TestSetup$1.protect(TestSetup.java:21) at junit.extensions.TestSetup.run(TestSetup.java:25) at org.apache.derbyTesting.junit.BaseTestSetup.run(BaseTestSetup.java:57) at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24) at junit.extensions.TestSetup$1.protect(TestSetup.java:21) at junit.extensions.TestSetup.run(TestSetup.java:25) at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24) at junit.extensions.TestSetup$1.protect(TestSetup.java:21) at junit.extensions.TestSetup.run(TestSetup.java:25) at org.apache.derbyTesting.junit.BaseTestSetup.run(BaseTestSetup.java:57) at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24) at junit.extensions.TestSetup$1.protect(TestSetup.java:21) at junit.extensions.TestSetup.run(TestSetup.java:25) at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24) at junit.extensions.TestSetup$1.protect(TestSetup.java:21) at junit.extensions.TestSetup.run(TestSetup.java:25) at org.apache.derbyTesting.junit.BaseTestSetup.run(BaseTestSetup.java:57) at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24) at junit.extensions.TestSetup$1.protect(TestSetup.java:21) at junit.extensions.TestSetup.run(TestSetup.java:25) FAILURES!!! Tests run: 2, Failures: 2, Errors: 0 real 0m6.508s user 0m0.000s sys 0m0.015s The existing test for logDevice jdbc url attribute are still in the old test harness java\testing\org\apache\derbyTesting\functionTests\tests\store\LogDeviceTest.java I can always go and do my testing for existence of readme files in that old harness test but it will be good to add the support for this attribute in junit framework.
          Mamta A. Satoor made changes -
          Issue & fix info High Value Fix [ 10422 ] High Value Fix,Release Note Needed [ 10422, 10101 ]
          Hide
          Mamta A. Satoor added a comment -

          Committed patch DERBY5232_patch7_diff.txt with revision 1407170 into the trunk. Now working on writing junit test

          Show
          Mamta A. Satoor added a comment - Committed patch DERBY5232_patch7_diff.txt with revision 1407170 into the trunk. Now working on writing junit test
          Mamta A. Satoor made changes -
          Attachment DERBY5232_patch7_diff.txt [ 12552491 ]
          Attachment DERBY5232_patch7_stat.txt [ 12552492 ]
          Hide
          Mamta A. Satoor added a comment -

          This patch uses OutputStreamWriter to write the readme files. If there are no further comments, I will go ahead and commit this.

          I am also working on writing junit test which will look for the 3 readme files after the database is created.

          Show
          Mamta A. Satoor added a comment - This patch uses OutputStreamWriter to write the readme files. If there are no further comments, I will go ahead and commit this. I am also working on writing junit test which will look for the 3 readme files after the database is created.
          Hide
          Knut Anders Hatlen added a comment -

          I don't think using Properties.store(OutputStream,String) for this is the right way if the intention is that the files should be translatable. The javadoc for that method says it will use ISO 8859-1 encoding, so it won't produce readable output in, say, Japanese or Chinese.

          A more straightforward way might be to wrap the OutputStream in an OutputStreamWriter, using the constructor that takes the name of the encoding, and call the write(String) method on the Writer.

          Show
          Knut Anders Hatlen added a comment - I don't think using Properties.store(OutputStream,String) for this is the right way if the intention is that the files should be translatable. The javadoc for that method says it will use ISO 8859-1 encoding, so it won't produce readable output in, say, Japanese or Chinese. A more straightforward way might be to wrap the OutputStream in an OutputStreamWriter, using the constructor that takes the name of the encoding, and call the write(String) method on the Writer.
          Mamta A. Satoor made changes -
          Attachment DERBY5232_patch6_diff.txt [ 12552209 ]
          Attachment DERBY5232_patch6_stat.txt [ 12552210 ]
          Hide
          Mamta A. Satoor added a comment -

          Knut, thanks for reviewing the last patch. I am submitting patch #6. This patch is same as the previous one except a prototype change in RawStore where we create the readme file for seg0 directory.

          In this patch, in RawStore, now I am mimicking what we do for storing services.properties ie write the warning text in the readme file using a Properties object. This gets rid of extra bytes at the beginning of the readme file with the earlier patch. One thing to note though is that with using the Properties object to write the text, the current date time stamp gets appended at the end of the warning text. I think having date time stamp should not be an issue.

          With this patch, I do not see any formatting errors with newline on my windows machine.

          If we are ok with using Properties object, then I will go ahead and make similar change to the code for readme files at the database level and at log directory.

          I am also working on writing a junit test which will check if the 3 readme files get created at the end of database creation. Please let me know if there is any feedback on using Properties object to write the text into readme files. Thanks

          Show
          Mamta A. Satoor added a comment - Knut, thanks for reviewing the last patch. I am submitting patch #6. This patch is same as the previous one except a prototype change in RawStore where we create the readme file for seg0 directory. In this patch, in RawStore, now I am mimicking what we do for storing services.properties ie write the warning text in the readme file using a Properties object. This gets rid of extra bytes at the beginning of the readme file with the earlier patch. One thing to note though is that with using the Properties object to write the text, the current date time stamp gets appended at the end of the warning text. I think having date time stamp should not be an issue. With this patch, I do not see any formatting errors with newline on my windows machine. If we are ok with using Properties object, then I will go ahead and make similar change to the code for readme files at the database level and at log directory. I am also working on writing a junit test which will check if the 3 readme files get created at the end of database creation. Please let me know if there is any feedback on using Properties object to write the text into readme files. Thanks
          Hide
          Knut Anders Hatlen added a comment -

          Thanks. The latest patch addresses all my previous comments, except the one about the extra bytes at the beginning of the files.

          Show
          Knut Anders Hatlen added a comment - Thanks. The latest patch addresses all my previous comments, except the one about the extra bytes at the beginning of the files.
          Mamta A. Satoor made changes -
          Attachment DERBY5232_patch5_diff.txt [ 12551878 ]
          Attachment DERBY5232_patch5_stat.txt [ 12551879 ]
          Hide
          Mamta A. Satoor added a comment -

          Hi Knut, hopefully this patch ddresses items from your previous comments except the one about "
          There are some garbage bytes at the beginning of the readme files. I think this is because the writeUTF() method writes length information before the actual text. See javadoc for java.io.DataOutput#writeUTF(). It would be good if the readme files were pure text files. " I am working on this. Thanks

          Show
          Mamta A. Satoor added a comment - Hi Knut, hopefully this patch ddresses items from your previous comments except the one about " There are some garbage bytes at the beginning of the readme files. I think this is because the writeUTF() method writes length information before the actual text. See javadoc for java.io.DataOutput#writeUTF(). It would be good if the readme files were pure text files. " I am working on this. Thanks
          Hide
          Knut Anders Hatlen added a comment -

          For the record, I also see the problem with missing files when running from classes, not just with the jars.

          The logic in RawStore.createDataWarningFile() seems to be backward. That is, it only attempts to create the file if it already exists.

          Also, I don't think it's correct to use actionCode = REGULAR_FILE_EXISTS_ACTION in privRandomAccessFile(), as that action will return a Boolean rather than a StorageRandomAccessFile and probably result in a ClassCastException.

          Some other nits:

          After fixing the typos, the asterisks around the text don't line up nicely anymore.

          There are some garbage bytes at the beginning of the readme files. I think this is because the writeUTF() method writes length information before the actual text. See javadoc for java.io.DataOutput#writeUTF(). It would be good if the readme files were pure text files.

          Show
          Knut Anders Hatlen added a comment - For the record, I also see the problem with missing files when running from classes, not just with the jars. The logic in RawStore.createDataWarningFile() seems to be backward. That is, it only attempts to create the file if it already exists. Also, I don't think it's correct to use actionCode = REGULAR_FILE_EXISTS_ACTION in privRandomAccessFile(), as that action will return a Boolean rather than a StorageRandomAccessFile and probably result in a ClassCastException. Some other nits: After fixing the typos, the asterisks around the text don't line up nicely anymore. There are some garbage bytes at the beginning of the readme files. I think this is because the writeUTF() method writes length information before the actual text. See javadoc for java.io.DataOutput#writeUTF(). It would be good if the readme files were pure text files.
          Hide
          Mamta A. Satoor added a comment - - edited

          Knut, I fixed the typo with revision 1403973. As for the behavior with the jars, I am debugging to see why the code path might be different when going through the jars rather than classes directly. Or it might be that there is an exception being thrown which we ignore in the catch block.

          Show
          Mamta A. Satoor added a comment - - edited Knut, I fixed the typo with revision 1403973. As for the behavior with the jars, I am debugging to see why the code path might be different when going through the jars rather than classes directly. Or it might be that there is an exception being thrown which we ignore in the catch block.
          Hide
          Knut Anders Hatlen added a comment -

          Thanks, Mamta. The spelling of "constitute" still isn't quite right. There's a "t" missing between the "u" and the "e".

          There still seems to be a problem with the generation of the files. I built jars from head of trunk and ran this in ij:

          ij version 10.10
          ij> connect 'jdbc:derby:db;create=true';
          ij> connect 'jdbc:derby:;shutdown=true';
          ERROR XJ015: Derby system shutdown.

          After this, there was a README in the top-level db directory, but none in db/seg0 or in db/log.

          Show
          Knut Anders Hatlen added a comment - Thanks, Mamta. The spelling of "constitute" still isn't quite right. There's a "t" missing between the "u" and the "e". There still seems to be a problem with the generation of the files. I built jars from head of trunk and ran this in ij: ij version 10.10 ij> connect 'jdbc:derby:db;create=true'; ij> connect 'jdbc:derby:;shutdown=true'; ERROR XJ015: Derby system shutdown. After this, there was a README in the top-level db directory, but none in db/seg0 or in db/log.
          Hide
          Mamta A. Satoor added a comment -

          Made 2 commits into trunk which will create 3 readme files, one at db directory level, one in "seg0" directory and one in "log" directory. The revision numbers are 1403611 and 1403807.

          Show
          Mamta A. Satoor added a comment - Made 2 commits into trunk which will create 3 readme files, one at db directory level, one in "seg0" directory and one in "log" directory. The revision numbers are 1403611 and 1403807.
          Mamta A. Satoor made changes -
          Fix Version/s 10.10.0.0 [ 12321550 ]
          Hide
          Mamta A. Satoor added a comment -

          I will work on putting privileged blocks around file creation. I believe that is the cause behind the failures. Knut, I will also work on your comments. thanks

          Show
          Mamta A. Satoor added a comment - I will work on putting privileged blocks around file creation. I believe that is the cause behind the failures. Knut, I will also work on your comments. thanks
          Hide
          Knut Anders Hatlen added a comment -

          Some post-commit review comments:

          Typos in the messages:

          CONSTITUES -> CONSTITUTE

          DATA(USER AND SYSTEM) -> add space before opening parenthesis

          ADDITING -> ADDING

          The fileReadMeDB file handles are closed twice in the normal (non-error) case. Although that's harmless (at least it should be after DERBY-5960), it's unnecessary code.

          Show
          Knut Anders Hatlen added a comment - Some post-commit review comments: Typos in the messages: CONSTITUES -> CONSTITUTE DATA(USER AND SYSTEM) -> add space before opening parenthesis ADDITING -> ADDING The fileReadMeDB file handles are closed twice in the normal (non-error) case. Although that's harmless (at least it should be after DERBY-5960 ), it's unnecessary code.
          Hide
          Knut Anders Hatlen added a comment -

          It looks like most of the tests fail with permission errors after the patch was committed.

          For example, here's a failure in the demo suite:

          http://dbtg.foundry.sun.com/derby/test/tinderbox_trunk16/jvm1.6/testing/testlog/SunOS-5.10_i86pc-i386/1403611-org.apache.derbyTesting.functionTests.tests.demo._Suite_diff.txt

          Caused by: java.security.AccessControlException: access denied (java.io.FilePermission /export/home/tmp/os136789/testingDerbyTinderBox/SunOS-5.10_i86pc-i386/org.apache.derbyTesting.functionTests.tests.demo._Suite/system/wombat/log/README_DONT_TOUCH_FILES.txt read)
          at java.security.AccessControlContext.checkPermission(AccessControlContext.java:374)
          at java.security.AccessController.checkPermission(AccessController.java:546)
          at java.lang.SecurityManager.checkPermission(SecurityManager.java:532)
          at java.lang.SecurityManager.checkRead(SecurityManager.java:871)
          at java.io.RandomAccessFile.<init>(RandomAccessFile.java:203)
          at org.apache.derby.impl.io.DirRandomAccessFile.<init>(Unknown Source)
          at org.apache.derby.impl.io.DirFile4.getRandomAccessFile(Unknown Source)
          at org.apache.derby.impl.store.raw.log.LogToFile.createLogDirectory(Unknown Source)
          at org.apache.derby.impl.store.raw.log.LogToFile.boot(Unknown Source)

          Show
          Knut Anders Hatlen added a comment - It looks like most of the tests fail with permission errors after the patch was committed. For example, here's a failure in the demo suite: http://dbtg.foundry.sun.com/derby/test/tinderbox_trunk16/jvm1.6/testing/testlog/SunOS-5.10_i86pc-i386/1403611-org.apache.derbyTesting.functionTests.tests.demo._Suite_diff.txt Caused by: java.security.AccessControlException: access denied (java.io.FilePermission /export/home/tmp/os136789/testingDerbyTinderBox/SunOS-5.10_i86pc-i386/org.apache.derbyTesting.functionTests.tests.demo._Suite/system/wombat/log/README_DONT_TOUCH_FILES.txt read) at java.security.AccessControlContext.checkPermission(AccessControlContext.java:374) at java.security.AccessController.checkPermission(AccessController.java:546) at java.lang.SecurityManager.checkPermission(SecurityManager.java:532) at java.lang.SecurityManager.checkRead(SecurityManager.java:871) at java.io.RandomAccessFile.<init>(RandomAccessFile.java:203) at org.apache.derby.impl.io.DirRandomAccessFile.<init>(Unknown Source) at org.apache.derby.impl.io.DirFile4.getRandomAccessFile(Unknown Source) at org.apache.derby.impl.store.raw.log.LogToFile.createLogDirectory(Unknown Source) at org.apache.derby.impl.store.raw.log.LogToFile.boot(Unknown Source)
          Mamta A. Satoor made changes -
          Attachment DERBY5232_patch4_diff.txt [ 12551026 ]
          Attachment DERBY5232_patch4_stat.txt [ 12551027 ]
          Hide
          Mamta A. Satoor added a comment -

          Here is another patch(DERBY5232_patch4_diff.txt) which is ready for commit. Some of the changes going in this patch are as follows. If there are no further comments, I will go ahead and commit it next week. As the next step, I will look at creating these readme files at the time of upgrade. Next step can be having these readme files either backed up at backup time and then restore them, or (re)create them at the restore time whether on not they were part of the backup process.

          1)Three readme files(README_DONT_TOUCH_FILES.txt) will be created. One at the database level, one in the "log" directory and another in the "seg0" directory.
          a)Content of the readme file at the database level are as follows

          1. *************************************************************************
          2. *** Do not touch files in this directory! ***
          3. *** FILES IN THIS DIRECTORY AND SUB-DIRECTORIES CONSTITUES DERBY ***
          4. *** DATABASE WHICH INCLUDES THE DATA(USER AND SYSTEM) AND THE ***
          5. *** NECESSARY FILES FOR DATABASE RECOVERY. ***
          6. *** EDITING, ADDITING OR DELETING ANY OF THESE FILES MAY CAUSE DATA ***
          7. *** CORRUPTION AND LEAVE THE DATABASE IN NON-RECOVERABLE STATE. ***
          8. *************************************************************************
            b)Content of the readme file at "seg0" directory are as follows
          9. *************************************************************************
          10. *** Do not touch files in this directory! ***
          11. *** FILES IN THIS DIRECTORY ARE USED BY THE DERBY DATABASE TO STORE ***
          12. *** USER AND SYSTEM DATA. EDITING, ADDING, OR DELETING FILES IN THIS ***
          13. *** DIRECTORY WILL CORRUPT THE ASSOCIATED DERBY DATABASE, AND WILL ***
          14. *** NOT BE RECOVERABLE. ***
          15. *************************************************************************
            c)Content of the readme file at "log" directory are as follows
          16. *************************************************************************
          17. *** Do not touch files in this directory! ***
          18. *** FILES IN THIS DIRECTORY ARE USED BY THE DERBY DATABASE RECOVERY ***
          19. *** SYSTEM. EDITING, ADDING OR DELETING FILES IN THIS DIRECTORY WILL ***
          20. *** CAUSE THE DERBY RECOVERY SYSTEM TO FAIL LEADING TO UNRECOVERABLE ***
          21. *** CORRUPT DATABASES. ***
          22. *************************************************************************
            2)I have replaced all the tabs with spaces and looked at the diff and it seems consistent with indentation. Please let me know if I have missed anything.
            3)I went ahead with duplicating the readme writing code in 3 places rather than adding a new method to StorageFactory.java interface because that puts the burden of implementing that method on atleast 5 classes right now which implement StorageFactory interface. Please let me know if the code duplication should be handled differntly.

          Kathey, you had good suggestion about pointing out attention to ,lck and service.properties and permission issues with multiple users in the log file. I can't recall offhand but if this is mentioned in some central place in our doc, but if it is, then maybe we can have a blurb in the 3 readme files to go that section of the doc to get more information about the files in the database directories.

          Knut, I have used writeUTF api to write into the 3 readme files. Can you please try this patch on your machine and see if you are able to view them with less now that they are in UTF-8 format?

          Show
          Mamta A. Satoor added a comment - Here is another patch(DERBY5232_patch4_diff.txt) which is ready for commit. Some of the changes going in this patch are as follows. If there are no further comments, I will go ahead and commit it next week. As the next step, I will look at creating these readme files at the time of upgrade. Next step can be having these readme files either backed up at backup time and then restore them, or (re)create them at the restore time whether on not they were part of the backup process. 1)Three readme files(README_DONT_TOUCH_FILES.txt) will be created. One at the database level, one in the "log" directory and another in the "seg0" directory. a)Content of the readme file at the database level are as follows ************************************************************************* *** Do not touch files in this directory! *** *** FILES IN THIS DIRECTORY AND SUB-DIRECTORIES CONSTITUES DERBY *** *** DATABASE WHICH INCLUDES THE DATA(USER AND SYSTEM) AND THE *** *** NECESSARY FILES FOR DATABASE RECOVERY. *** *** EDITING, ADDITING OR DELETING ANY OF THESE FILES MAY CAUSE DATA *** *** CORRUPTION AND LEAVE THE DATABASE IN NON-RECOVERABLE STATE. *** ************************************************************************* b)Content of the readme file at "seg0" directory are as follows ************************************************************************* *** Do not touch files in this directory! *** *** FILES IN THIS DIRECTORY ARE USED BY THE DERBY DATABASE TO STORE *** *** USER AND SYSTEM DATA. EDITING, ADDING, OR DELETING FILES IN THIS *** *** DIRECTORY WILL CORRUPT THE ASSOCIATED DERBY DATABASE, AND WILL *** *** NOT BE RECOVERABLE. *** ************************************************************************* c)Content of the readme file at "log" directory are as follows ************************************************************************* *** Do not touch files in this directory! *** *** FILES IN THIS DIRECTORY ARE USED BY THE DERBY DATABASE RECOVERY *** *** SYSTEM. EDITING, ADDING OR DELETING FILES IN THIS DIRECTORY WILL *** *** CAUSE THE DERBY RECOVERY SYSTEM TO FAIL LEADING TO UNRECOVERABLE *** *** CORRUPT DATABASES. *** ************************************************************************* 2)I have replaced all the tabs with spaces and looked at the diff and it seems consistent with indentation. Please let me know if I have missed anything. 3)I went ahead with duplicating the readme writing code in 3 places rather than adding a new method to StorageFactory.java interface because that puts the burden of implementing that method on atleast 5 classes right now which implement StorageFactory interface. Please let me know if the code duplication should be handled differntly. Kathey, you had good suggestion about pointing out attention to ,lck and service.properties and permission issues with multiple users in the log file. I can't recall offhand but if this is mentioned in some central place in our doc, but if it is, then maybe we can have a blurb in the 3 readme files to go that section of the doc to get more information about the files in the database directories. Knut, I have used writeUTF api to write into the 3 readme files. Can you please try this patch on your machine and see if you are able to view them with less now that they are in UTF-8 format?
          Hide
          Knut Anders Hatlen added a comment -

          The non-ASCII character may also have been garbled by MessageBuilder when it converts messages.xml (which is encoded in UTF-8) to messages_en.properties (which is supposed to be encoded in ISO-8859-1, with multi-byte UTF-8 characters encoded as a Unicode escape sequence). I don't think MessageBuilder currently does that re-encoding.

          Show
          Knut Anders Hatlen added a comment - The non-ASCII character may also have been garbled by MessageBuilder when it converts messages.xml (which is encoded in UTF-8) to messages_en.properties (which is supposed to be encoded in ISO-8859-1, with multi-byte UTF-8 characters encoded as a Unicode escape sequence). I don't think MessageBuilder currently does that re-encoding.
          Hide
          Mamta A. Satoor added a comment -

          Thanks for the tip, Knut. I added UTF-8 character ♉ to messages.xml temporarily but it got printed as ? in the readme file. This is probably happening because of the call use to get the message from messages.xml. The call is MessageService.getTextMessage(MessageId.README_AT_DB_LEVEL). This method probably converts the non-ascii characters into ?. I am looking to see if there is another api I can use to get the UTF-8 characters as they are using the MessageId.README_AT_DB_LEVEL.

          Show
          Mamta A. Satoor added a comment - Thanks for the tip, Knut. I added UTF-8 character ♉ to messages.xml temporarily but it got printed as ? in the readme file. This is probably happening because of the call use to get the message from messages.xml. The call is MessageService.getTextMessage(MessageId.README_AT_DB_LEVEL). This method probably converts the non-ascii characters into ?. I am looking to see if there is another api I can use to get the UTF-8 characters as they are using the MessageId.README_AT_DB_LEVEL.
          Hide
          Knut Anders Hatlen added a comment -

          Since the files only contain characters in the 7-bit ASCII range, UTF-8 and ASCII are indistinguishable. To see if it's actually UTF-8 and not ASCII that is being used, you could add a non-ASCII character to the message temporarily and see if it is detected as UTF-8 then.

          Show
          Knut Anders Hatlen added a comment - Since the files only contain characters in the 7-bit ASCII range, UTF-8 and ASCII are indistinguishable. To see if it's actually UTF-8 and not ASCII that is being used, you could add a non-ASCII character to the message temporarily and see if it is detected as UTF-8 then.
          Hide
          Mamta A. Satoor added a comment -

          To solve the UTF-8 vs UTF-16 problem mentioned by Knut(1), am I right that I should use
          randomAccessFile.writeUTF(fileContent);
          rather than
          randomAccessFile.writeChars(fileContent);
          How would I verify that we the writeUTF call, the file has indeed been saved in UTF-8 format. I thought if I opened the file in notepad and used "save as", in the encoding dropdown, I will see UTF-8 rather than ASCII, but I still see ASCII even when the file gets it contents with writeUTF call rather than writeChars call. Thanks

          (1)The files were stored in UTF-16, so less complained they were binary files and wouldn't open them. They'd probably be easier to access if they were stored in UTF-8.

          Show
          Mamta A. Satoor added a comment - To solve the UTF-8 vs UTF-16 problem mentioned by Knut(1), am I right that I should use randomAccessFile.writeUTF(fileContent); rather than randomAccessFile.writeChars(fileContent); How would I verify that we the writeUTF call, the file has indeed been saved in UTF-8 format. I thought if I opened the file in notepad and used "save as", in the encoding dropdown, I will see UTF-8 rather than ASCII, but I still see ASCII even when the file gets it contents with writeUTF call rather than writeChars call. Thanks (1)The files were stored in UTF-16, so less complained they were binary files and wouldn't open them. They'd probably be easier to access if they were stored in UTF-8.
          Mamta A. Satoor made changes -
          Attachment DERBY5232_patch3_diff.txt [ 12550641 ]
          Attachment DERBY5232_patch3_stat.txt [ 12550642 ]
          Hide
          Mamta A. Satoor added a comment -

          Another updated patch. It is NOT ready for commit and has indentation issues and can use more comments but I wanted to share couple changes made in this patch compared to the last one.

          1)I talked to Mike offline about getting hold of storagefactory in the "access" code in order to create readme file in the seg0 directory. Mike suggested that we create a new method(say createDataWarningFile) in RawStoreFactory.java interface to create a readme file. RawStore implements this interface and it will create the readme file using storagefactory. RAMAccessManager can then call rawstore.createDataWarningFile to create the readme file in seg0 directory

          2)To address the code duplication brought up by Knut,
          I agree with that and was planning on doing something about it. Currently, we create 3 readme files and the code to do is pretty much the same other than the directory path and message id used to load the content into the newly created file. I thought of adding a new method to StorageFactory.java interface, where a new file will be created in the passed directory and the new file will get data written into it passed as a parameter to the new method. The new method would look as follows
          public StorageFile newStorageFile( String directoryName,
          String fileName,
          String fileContent);
          But the problem with this approach is that StorageFactory interface gets implemented by 5 classes and hence I am having to put the method implementation in all those 5 classes. I think rather than putting the burden on StorageFactory implementor to implement this new method, we should continue to have the code duplication, since that code is not too big. I am open to suggestions on this one, though. The new patch attached shows the new method in StorageFactory and it's implementation by 5 different classes.

          3)Also, I think the try catch finally code for creating the new file and loading data into it can be simplified to following. Let me know if I am missing something in this code optimization
          Code in the newly attached patch is as follows
          + StorageRandomAccessFile randomAccessFile=null;
          + try

          { + randomAccessFile = sf.getRandomAccessFile("rw"); + randomAccessFile.writeChars(fileContent); + }
          + catch (IOException ioe)
          + {
          + try
          + { + if (randomAccessFile != null) + randomAccessFile.close(); + }
          + catch (IOException ioe2)
          + { + randomAccessFile = null; + }
          + }
          + finally
          + {
          + try
          + { + if (randomAccessFile != null) + randomAccessFile.close(); + }
          + catch (IOException ioe)
          + { + // Ignore exception on close + randomAccessFile = null; + }
          + }
          + return sf;

          Instead, we can write it as follows
          Another updated patch. It is NOT ready for commit and has indentation issues and can use more comments but I wanted to share couple changes made in this patch compared to the last one.

          1)I talked to Mike offline about getting hold of storagefactory in the "access" code in order to create readme file in the seg0 directory. Mike suggested that we create a new method(say createDataWarningFile) in RawStoreFactory.java interface to create a readme file. RawStore implements this interface and it will create the readme file using storagefactory. RAMAccessManager can then call rawstore.createDataWarningFile to create the readme file in seg0 directory

          2)To address the code duplication brought up by Knut,
          I agree with that and was planning on doing something about it. Currently, we create 3 readme files and the code to do is pretty much the same other than the directory path and message id used to load the content into the newly created file. I thought of adding a new method to StorageFactory.java interface, where a new file will be created in the passed directory and the new file will get data written into it passed as a parameter to the new method. The new method would look as follows
          public StorageFile newStorageFile( String directoryName,
          String fileName,
          String fileContent);
          But the problem with this approach is that StorageFactory interface gets implemented by 5 classes and hence I am having to put the method implementation in all those 5 classes. I think rather than putting the burden on StorageFactory implementor to implement this new method, we should continue to have the code duplication, since that code is not too big. I am open to suggestions on this one, though. The new patch attached shows the new method in StorageFactory and it's implementation by 5 different classes.

          3)Also, I think the try catch finally code for creating the new file and loading data into it can be simplified to following. Let me know if I am missing something in this code optimization
          Code in the newly attached patch is as follows
          + StorageRandomAccessFile randomAccessFile=null;
          + try { + randomAccessFile = sf.getRandomAccessFile("rw"); + randomAccessFile.writeChars(fileContent); + }

          + catch (IOException ioe)
          +

          { + }

          + finally
          + {
          + try
          +

          { + if (randomAccessFile != null) + randomAccessFile.close(); + }

          + catch (IOException ioe)
          +

          { + // Ignore exception on close + randomAccessFile = null; + }

          + }
          + return sf;

          Basically, I have removed the code from inside the catch block since finally does the exact same thing.

          As always, appreciate any feedback on the patch. Thanks.

          Show
          Mamta A. Satoor added a comment - Another updated patch. It is NOT ready for commit and has indentation issues and can use more comments but I wanted to share couple changes made in this patch compared to the last one. 1)I talked to Mike offline about getting hold of storagefactory in the "access" code in order to create readme file in the seg0 directory. Mike suggested that we create a new method(say createDataWarningFile) in RawStoreFactory.java interface to create a readme file. RawStore implements this interface and it will create the readme file using storagefactory. RAMAccessManager can then call rawstore.createDataWarningFile to create the readme file in seg0 directory 2)To address the code duplication brought up by Knut, I agree with that and was planning on doing something about it. Currently, we create 3 readme files and the code to do is pretty much the same other than the directory path and message id used to load the content into the newly created file. I thought of adding a new method to StorageFactory.java interface, where a new file will be created in the passed directory and the new file will get data written into it passed as a parameter to the new method. The new method would look as follows public StorageFile newStorageFile( String directoryName, String fileName, String fileContent); But the problem with this approach is that StorageFactory interface gets implemented by 5 classes and hence I am having to put the method implementation in all those 5 classes. I think rather than putting the burden on StorageFactory implementor to implement this new method, we should continue to have the code duplication, since that code is not too big. I am open to suggestions on this one, though. The new patch attached shows the new method in StorageFactory and it's implementation by 5 different classes. 3)Also, I think the try catch finally code for creating the new file and loading data into it can be simplified to following. Let me know if I am missing something in this code optimization Code in the newly attached patch is as follows + StorageRandomAccessFile randomAccessFile=null; + try { + randomAccessFile = sf.getRandomAccessFile("rw"); + randomAccessFile.writeChars(fileContent); + } + catch (IOException ioe) + { + try + { + if (randomAccessFile != null) + randomAccessFile.close(); + } + catch (IOException ioe2) + { + randomAccessFile = null; + } + } + finally + { + try + { + if (randomAccessFile != null) + randomAccessFile.close(); + } + catch (IOException ioe) + { + // Ignore exception on close + randomAccessFile = null; + } + } + return sf; Instead, we can write it as follows Another updated patch. It is NOT ready for commit and has indentation issues and can use more comments but I wanted to share couple changes made in this patch compared to the last one. 1)I talked to Mike offline about getting hold of storagefactory in the "access" code in order to create readme file in the seg0 directory. Mike suggested that we create a new method(say createDataWarningFile) in RawStoreFactory.java interface to create a readme file. RawStore implements this interface and it will create the readme file using storagefactory. RAMAccessManager can then call rawstore.createDataWarningFile to create the readme file in seg0 directory 2)To address the code duplication brought up by Knut, I agree with that and was planning on doing something about it. Currently, we create 3 readme files and the code to do is pretty much the same other than the directory path and message id used to load the content into the newly created file. I thought of adding a new method to StorageFactory.java interface, where a new file will be created in the passed directory and the new file will get data written into it passed as a parameter to the new method. The new method would look as follows public StorageFile newStorageFile( String directoryName, String fileName, String fileContent); But the problem with this approach is that StorageFactory interface gets implemented by 5 classes and hence I am having to put the method implementation in all those 5 classes. I think rather than putting the burden on StorageFactory implementor to implement this new method, we should continue to have the code duplication, since that code is not too big. I am open to suggestions on this one, though. The new patch attached shows the new method in StorageFactory and it's implementation by 5 different classes. 3)Also, I think the try catch finally code for creating the new file and loading data into it can be simplified to following. Let me know if I am missing something in this code optimization Code in the newly attached patch is as follows + StorageRandomAccessFile randomAccessFile=null; + try { + randomAccessFile = sf.getRandomAccessFile("rw"); + randomAccessFile.writeChars(fileContent); + } + catch (IOException ioe) + { + } + finally + { + try + { + if (randomAccessFile != null) + randomAccessFile.close(); + } + catch (IOException ioe) + { + // Ignore exception on close + randomAccessFile = null; + } + } + return sf; Basically, I have removed the code from inside the catch block since finally does the exact same thing. As always, appreciate any feedback on the patch. Thanks.
          Hide
          Knut Anders Hatlen added a comment -

          The double closing of the readme file causes a NullPointerException when creating an in-memory database. I think that's a problem in VirtualRandomAccessFile.close(), as close() should not fail just because it's called more than once, so I've filed a separate bug for it, DERBY-5960.

          Show
          Knut Anders Hatlen added a comment - The double closing of the readme file causes a NullPointerException when creating an in-memory database. I think that's a problem in VirtualRandomAccessFile.close(), as close() should not fail just because it's called more than once, so I've filed a separate bug for it, DERBY-5960 .
          Knut Anders Hatlen made changes -
          Link This issue is related to DERBY-5960 [ DERBY-5960 ]
          Hide
          Knut Anders Hatlen added a comment -

          Hi Mamta,

          I don't know why the newlines disappear. They seemed to be there when I tested the patch. Maybe it's a Windows vs Unix line ending issue? I had another problem, though. The files were stored in UTF-16, so less complained they were binary files and wouldn't open them. They'd probably be easier to access if they were stored in UTF-8.

          Some stylistic comments:

          • The indentation used in the new code varies between pure spaces, pure tabs and mixed tabs/spaces. It would be easier to see the structure of the code in the patch with a consistent indentation style.
          • The code that produces the readme files is repeated three times. It would be good if it could be refactored so that there's a single implementation.
          • When writing the readme file, the file handle is closed inside the try block, in the catch block and in the finally clause. In the common case, it will be closed twice. It's probably sufficient to close it in the finally clause.
          Show
          Knut Anders Hatlen added a comment - Hi Mamta, I don't know why the newlines disappear. They seemed to be there when I tested the patch. Maybe it's a Windows vs Unix line ending issue? I had another problem, though. The files were stored in UTF-16, so less complained they were binary files and wouldn't open them. They'd probably be easier to access if they were stored in UTF-8. Some stylistic comments: The indentation used in the new code varies between pure spaces, pure tabs and mixed tabs/spaces. It would be easier to see the structure of the code in the patch with a consistent indentation style. The code that produces the readme files is repeated three times. It would be good if it could be refactored so that there's a single implementation. When writing the readme file, the file handle is closed inside the try block, in the catch block and in the finally clause. In the common case, it will be closed twice. It's probably sufficient to close it in the finally clause.
          Mamta A. Satoor made changes -
          Attachment DERBY5232_patch2_diff.txt [ 12550410 ]
          Attachment DERBY5232_patch2_stat.txt [ 12550411 ]
          Hide
          Mamta A. Satoor added a comment -

          I have another patch which has following changes compared to the first patch
          1)It does not hard code the readme contents inside the java file. Instead, it uses existing mechanics to store that text inside messages.xml so it will get translated.
          2)The code now handles StandardException as pointed out by Knut.
          3)I have named the readme file as README_DONT_TOUCH_FILES.txt
          4)I have been able to create README_DONT_TOUCH_FILES.txt at the top level(db level) and at log level. But I have not been able to find yet the place in the code to create a README_DONT_TOUCH_FILES.txt at seg level and will greatly appreciate help here. I thought the correct location for creating this file in the code will be in store.access.RAMAccessManager:boot() method right
          after
          xactProperties = new PropertyConglomerate(tc, create, startParams, pf);
          The above line creates seg0 directory and creates the first heap file in it. And it seems like, right after the seg0 directory is created, we should create the README_DONT_TOUCH_FILES.txt but to create a file, we need access to StorageFactory which has the new method newStorageFile() to create a new file and I do not know how to get hold of StorageFactory inside RAMAccessManager. It is possible that I am looking at wrong place for adding the code. Would appreciate help from someone more familiar with this part of the code.
          5)Even though I have added the text is messages.xml with new lines, when it gets written into README_DONT_TOUCH_FILES.txt, I find that the newlines are lost and everything is added as one long line. I am not sure yet what I could be doing wrong.
          <msg>
          <name>M005</name>
          <text>

          1. *************************************************************************
          2. *** Do not touch files in this directory! ***
          3. *** CHANGING THE CONTENT OF THIS DIRECTORY MAY CAUSE DATA CORRUPTION. ***
          4. *************************************************************************</text>
            <comment>Translators: Please translate the ALL CAPS words.</comment>
            </msg>

          I still need to add more comments but wanted to put the patch out for review and for help with couple items I haven't figured out yet. Thanks.

          Show
          Mamta A. Satoor added a comment - I have another patch which has following changes compared to the first patch 1)It does not hard code the readme contents inside the java file. Instead, it uses existing mechanics to store that text inside messages.xml so it will get translated. 2)The code now handles StandardException as pointed out by Knut. 3)I have named the readme file as README_DONT_TOUCH_FILES.txt 4)I have been able to create README_DONT_TOUCH_FILES.txt at the top level(db level) and at log level. But I have not been able to find yet the place in the code to create a README_DONT_TOUCH_FILES.txt at seg level and will greatly appreciate help here. I thought the correct location for creating this file in the code will be in store.access.RAMAccessManager:boot() method right after xactProperties = new PropertyConglomerate(tc, create, startParams, pf); The above line creates seg0 directory and creates the first heap file in it. And it seems like, right after the seg0 directory is created, we should create the README_DONT_TOUCH_FILES.txt but to create a file, we need access to StorageFactory which has the new method newStorageFile() to create a new file and I do not know how to get hold of StorageFactory inside RAMAccessManager. It is possible that I am looking at wrong place for adding the code. Would appreciate help from someone more familiar with this part of the code. 5)Even though I have added the text is messages.xml with new lines, when it gets written into README_DONT_TOUCH_FILES.txt, I find that the newlines are lost and everything is added as one long line. I am not sure yet what I could be doing wrong. <msg> <name>M005</name> <text> ************************************************************************* *** Do not touch files in this directory! *** *** CHANGING THE CONTENT OF THIS DIRECTORY MAY CAUSE DATA CORRUPTION. *** *************************************************************************</text> <comment>Translators: Please translate the ALL CAPS words.</comment> </msg> I still need to add more comments but wanted to put the patch out for review and for help with couple items I haven't figured out yet. Thanks.
          Hide
          Knut Anders Hatlen added a comment -

          Yes, messages.xml is the right place. They shouldn't use SQL states, but instead four letter message ids. See the constants in org.apache.derby.shared.common.reference.MessageId interface for examples. The SERVICE_PROPERTIES_DONT_EDIT message with id M001 is similar to the messages we want to add in this issue.

          Show
          Knut Anders Hatlen added a comment - Yes, messages.xml is the right place. They shouldn't use SQL states, but instead four letter message ids. See the constants in org.apache.derby.shared.common.reference.MessageId interface for examples. The SERVICE_PROPERTIES_DONT_EDIT message with id M001 is similar to the messages we want to add in this issue.
          Hide
          Mamta A. Satoor added a comment -

          Is messages.xml where the content of the 3 readme files should go? I have added SQL messages in the past in messages.xml but not sure where the readme content should go so the contents will get translated. If they do go in messages.xml, then do we just use some dummy sql states for them so we can use the existing mechanics to reference them in the code? Thanks

          Show
          Mamta A. Satoor added a comment - Is messages.xml where the content of the 3 readme files should go? I have added SQL messages in the past in messages.xml but not sure where the readme content should go so the contents will get translated. If they do go in messages.xml, then do we just use some dummy sql states for them so we can use the existing mechanics to reference them in the code? Thanks
          Hide
          Kathey Marsden added a comment -

          I think Mike's recommendations sound perfect for file names, contents (except for typo: Ediiting) and locations. I think three files would be good as I have seen in the past misguided instructions telling people to rm log/* or rm -rf log without looking to see what's there. Mike, can you recommend wording for the top level file. Perhaps in addition to mentioning the log and seg0 directory, it could also mention that service.properties contains critical information about the format of the database and that the .lck files prevent dual boot and corruption of the database.

          Often when users embark down the deletion path, especially when it starts with lock files, I find it is because of permission issues where multiple users have booted (and perhaps modified) the database with inconsistent permissions. What should have been done in those instances was the update the file permissions to be consistent and what they want, but I don't know if it is good to explain that in the top level file or if that is too much information.

          Show
          Kathey Marsden added a comment - I think Mike's recommendations sound perfect for file names, contents (except for typo: Ediiting) and locations. I think three files would be good as I have seen in the past misguided instructions telling people to rm log/* or rm -rf log without looking to see what's there. Mike, can you recommend wording for the top level file. Perhaps in addition to mentioning the log and seg0 directory, it could also mention that service.properties contains critical information about the format of the database and that the .lck files prevent dual boot and corruption of the database. Often when users embark down the deletion path, especially when it starts with lock files, I find it is because of permission issues where multiple users have booted (and perhaps modified) the database with inconsistent permissions. What should have been done in those instances was the update the file permissions to be consistent and what they want, but I don't know if it is good to explain that in the top level file or if that is too much information.
          Hide
          Mike Matrigali added a comment -

          I second that the text located in the file should be generated from messages file so that it can be translated.

          I think it would be good if a file went into the log directory and into the seg0 directory. Users can locate the log directory on a different device so depending on implementation no note would
          be associated with the log files if the file is not in the actual directory. So maybe 3 files, one in seg0, one in log, and one in the parent directory.

          It would be good if a backup/restore either copied the file or generated it to get the same message to users looking at the backup and possibly corrupting it.

          How about README_DONT_TOUCH_FILES.txt for file names?

          As to content, seems like we should take the chance to be a little more verbose. Something like:

          For file in the log directory:
          Do not touch files in this directory!

          Files in this directory are used by the Derby database recovery system. Ediiting, adding or deleting files in this directory will cause the Derby recovery system to fail leading to unrecoverable corrupt databases.

          For the file in the seg0 directory:
          Do not touch files in this directory!

          Files in this directory are used by the Derby database to store user and system data. Ediiting, adding or deleting files in this directory will corrupt the associated Derby database, and will not
          be recoverable.

          Show
          Mike Matrigali added a comment - I second that the text located in the file should be generated from messages file so that it can be translated. I think it would be good if a file went into the log directory and into the seg0 directory. Users can locate the log directory on a different device so depending on implementation no note would be associated with the log files if the file is not in the actual directory. So maybe 3 files, one in seg0, one in log, and one in the parent directory. It would be good if a backup/restore either copied the file or generated it to get the same message to users looking at the backup and possibly corrupting it. How about README_DONT_TOUCH_FILES.txt for file names? As to content, seems like we should take the chance to be a little more verbose. Something like: For file in the log directory: Do not touch files in this directory! Files in this directory are used by the Derby database recovery system. Ediiting, adding or deleting files in this directory will cause the Derby recovery system to fail leading to unrecoverable corrupt databases. For the file in the seg0 directory: Do not touch files in this directory! Files in this directory are used by the Derby database to store user and system data. Ediiting, adding or deleting files in this directory will corrupt the associated Derby database, and will not be recoverable.
          Hide
          Knut Anders Hatlen added a comment -

          The run() method in the privileged block only closes the file if it is successful or it fails with an IOException. But it is declared as throws StandardException, and closing is not handled if it actually throws a SE, I think.

          Show
          Knut Anders Hatlen added a comment - The run() method in the privileged block only closes the file if it is successful or it fails with an IOException. But it is declared as throws StandardException, and closing is not handled if it actually throws a SE, I think.
          Hide
          Knut Anders Hatlen added a comment -

          As to the name of the file, README might stand out more in a directory listing, and that name might already be familiar as a warning sign to many users.

          The message text should probably go into messages.xml so that it can be translated.

          Show
          Knut Anders Hatlen added a comment - As to the name of the file, README might stand out more in a directory listing, and that name might already be familiar as a warning sign to many users. The message text should probably go into messages.xml so that it can be translated.
          Hide
          Kim Haase added a comment -

          Thanks, Mamta – it's a very good idea to provide this.

          I'd suggest a correction to the text of the file itself: please change

          fileSternReadMeDB.writeChars("Don't not touch any file in this directory");

          to

          fileSternReadMeDB.writeChars("Do not touch any file in this directory");

          Show
          Kim Haase added a comment - Thanks, Mamta – it's a very good idea to provide this. I'd suggest a correction to the text of the file itself: please change fileSternReadMeDB.writeChars("Don't not touch any file in this directory"); to fileSternReadMeDB.writeChars("Do not touch any file in this directory");
          Mamta A. Satoor made changes -
          Attachment DERBY5232_patch1_diff.txt [ 12549257 ]
          Attachment DERBY5232_patch1_stat.txt [ 12549258 ]
          Hide
          Mamta A. Satoor added a comment -

          Here is a prototype patch for creating a readme file at database creation time. This file will be at the same level as db.lck/services.properties. For now, I have named it DoNotTouchAnyFile.txt and the current content of the file are hard coded in the file as "Don't not touch any file in this directory". We probably want to name the file differently and rephrase the content as something different. Will appreciate feedback on that. Also, I haven't looked at possibly backing up and restoring this file during backup and restore. Not sure if we want this file to be included as part of backup/restore.

          Show
          Mamta A. Satoor added a comment - Here is a prototype patch for creating a readme file at database creation time. This file will be at the same level as db.lck/services.properties. For now, I have named it DoNotTouchAnyFile.txt and the current content of the file are hard coded in the file as "Don't not touch any file in this directory". We probably want to name the file differently and rephrase the content as something different. Will appreciate feedback on that. Also, I haven't looked at possibly backing up and restoring this file during backup and restore. Not sure if we want this file to be included as part of backup/restore.
          Mamta A. Satoor made changes -
          Field Original Value New Value
          Assignee Mamta A. Satoor [ mamtas ]
          Kathey Marsden created issue -

            People

            • Assignee:
              Mamta A. Satoor
              Reporter:
              Kathey Marsden
            • Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development