h2 database update to 1.3.168, improved handling of database exceptions #81

Closed
wants to merge 13 commits into
from

Projects

None yet

3 participants

@valib

and some cosmetic changes. Simplified previous commit

@valib valib h2 database update to 1.3.168, improved handling of database exceptions
and some cosmetic changes. Simplified previous commit
83704ce
@taconaut
PS3 Media Server member

I've tested this on win7 with a db created with 1.60 and it didn't work.

// Check whether the database is not severely damaged, corrupt or wrong version
try {
   conn = getConnection();
} catch (SQLException se) { // Connection can't be established, so delete the database
   FileUtils.deleteRecursive(dbDir, false);
   if (!FileUtils.exists(dbDir)){
      LOGGER.debug("Damaged, corrupted or wrong version database deleted");
   } else 
   LOGGER.info("Damaged database can't be deleted. Stop the program and delete it manually");
} finally {
   close(conn);
}

Getting the connection succeeds (no exception).

rs = stmt.executeQuery("SELECT count(*) FROM FILES");

fails with exception having error code 42102 (which is being masked)

Then it starts creating the tables (which doesn't result in an error).
When starting a scan after all this, pms shows it's scanning (stop button on magnifying glass) but the size of the DB files doesn't change.

@valib

Thanks for input.
I've tested it also on win7 and it worked. For the database version 1.70 the first connection raised exception 90048 which means the file header of a database files (*.db) does not match the expected version, or is corrupted. The main problem for the last version of database is that the name of the main database file was changed from previous *.data.db to *.h2.db. I will retest it again or use my previous idea to check the database file name.

@taconaut
PS3 Media Server member

I don't see any h2.db file in the database folder. The main problem I encounter is that getConnection() doesn't fail (and thus the db files aren't being deleted) but the connection is not functional.

To test, I've installed a fresh pms1.60 -> delete db folder -> launch pms1.60 -> enable cache -> save -> scan files (let it run 5min) -> close pms1.60 -> launch pms (head of valib:db_exceptions-handling-v2) from eclipse. Then what's described above happened.

@valib

I am now testing it and it works. Tested with database 1.54.0, 1.60.0 and 1.70.0, WIN7 32b, Java ver. 1.7.0_04.I also changed h2 database to ver. 1.3.167 (last stable) and everything is OK. I have no idea what's going on.

The h2.db file can't be seen because connection for you was succesfull and the file will be created automatically after the folder will be deleted.

@valib

Now the code is checking the database name and old version is forced to delete.

@taconaut
PS3 Media Server member

After re-testing on win7, it worked fine. What puzzles me is that this time I got the exception and didn't need the 'workaround' to check the 'old' db file name.

I'd suggest some small adjustments for the logged messages. Raise both by one level and talk about cache rather then db as this is the used terminology for users.
Change LOGGER.debug("Damaged, corrupted or wrong version database deleted"); to LOGGER.info("The cache has been deleted because it was corrupt or had the wrong version");
Change LOGGER.info("Damaged database can't be deleted. Stop the program and delete it manually"); to LOGGER.warn("Damaged cache can't be deleted. Stop the program and delete the folder <db_foder_path> manually");

@taconaut taconaut commented on an outdated diff Aug 31, 2012
src/main/java/net/pms/dlna/DLNAMediaDatabase.java
@@ -135,22 +160,16 @@ public void init(boolean force) {
version = rs.getString(1);
}
} catch (SQLException se) {
- logger.debug("Database not created or corrupted");
+ if (se.getErrorCode() != 42102) // Don't log exception "Table "FILES" not found" which will be corrected in following step
@taconaut
taconaut Aug 31, 2012

Please always put brackets { }, also for one-liners.

@valib

Hi taconaut,

thanks for your suggestins. Regarding the naming "database" or "cache" I am a little bit confused by the current code because both names are used. I think that better could be used only "database" because the cache is its secondary function. Standard cache doesn't have sorting functions of recorded files, is usually not accessible to users and is used only to speed up the programme. What's more the class is DLNAMediaDatabase not cache and "database" is used e.g. at lines 102, 103.

@taconaut
PS3 Media Server member

I'd just like to be coherent for the end user. The terminology in the GUI has been changed not that long ago to 'cache', I'd say we can stick with that, even though the 'cache folder' is more of a 'media library folder'.
That being said, I intend to work on pms-mlx again which will replace the entire existing caching mechanism.

@valib

I change the names but I propose to change both cache and database names in the whole programme to e.g. "cacheDatabase". Anyway I support to change to the pms-mlx database version.

@chocolateboy
PS3 Media Server member

No thanks: "cache" is fine.

@taconaut
PS3 Media Server member

Don't forget the headless servers, they also need a good message in the logs, at the appropriate level. Not sure what happens when trying to open a graphical component.

@chocolateboy
We've been talking about notification queues (https://github.com/taconaut/pms-mlx/tree/master/core/src/main/java/net/pms/notifications). This would be the perfect use case.

@valib

Sorry I have forgotten that the headless server exists. :-)

@valib

I hope that this was the last change

@valib

Never say never.

@valib

I hope it works.

@valib

Sorry. I had to revert the last commit published by accident.

@taconaut
PS3 Media Server member

Cheers and sorry for being a bit of a pain but I'd rather have everything done by the time of merging. There are two things left.

1) Messages shown to the user should be localizable:
Create a new entry in messages.properties like
DLNAMediaDatabase.5=Damaged cache has to be deleted but the program couldn't do it.\nStop the program and delete the folder %s manually.
and using it with String.Format(Messages.getString("DLNAMediaDatabase.5"), dbDir)

2) As I'm not sure what happens when trying to show a message box on a headless server (wait indefinitely for an OK click on an invisible message box? exception? application crash?), I'd rather secure its execution with if(!java.awt.GraphicsEnvironment.isHeadless()) { ... }
This lets us use the actual PMS frame as parent as well PMS.get().getFrame() as we are sure there is one.
Is anyone able to test this on a headless server?

@valib

Thanks for your points and don't apologize to correct my work. I appreciate that.

@taconaut taconaut added a commit that closed this pull request Sep 19, 2012
@taconaut taconaut Manually merged pull request 'h2 database update to 1.3.168, improved…
… handling of database exceptions' as it wasn't applicable without modifications (closes #81)

Updated changelog
3b8b0df
@taconaut taconaut closed this in 3b8b0df Sep 19, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment