Increase checksum noting and matching for imports. #890

mtbc · 2013-03-15T14:56:32Z

To test make sure that different import/upload paths still work, including archiving during import. For real files in the server's local filesystem, if they have a corresponding entry in the originalfile table in the DB then make sure that the SHA1 there is set correctly.

Makes progress on http://trac.openmicroscopy.org.uk/ome/ticket/2581

mtbc · 2013-03-15T19:18:35Z

components/tools/OmeroPy/src/omero/util/populate_roi.py

@@ -632,6 +632,7 @@ def update_table(self, columns):
            column_report[column.name] = len(column.values)
        log.debug("Column report: %r" % column_report)
        self.table.addData(columns)
+        self.table.close()


causes SHA1 to be written for the table file

joshmoore · 2013-03-18T07:06:28Z

Adding exclude label:

/home/hudson/hudson/workspace/OMERO-merge-stable/src/components/blitz/src/ome/formats/OMEROMetadataStoreClient.java:1854: cannot find symbol
symbol  : method copyOfRange(byte[],int,int)
location: class java.util.Arrays
                    hasher.putBytes(rlen == buf.length ? buf : Arrays.copyOfRange(buf, 0, rlen));

mtbc · 2013-03-18T08:12:25Z

That's interesting. I'm guessing that Eclipse's 1.5 compliance checking goes as far as the core language but doesn't do anything clever with APIs, then. I'll have to watch out for that. Of course, I'll provide a fix.

bpindelski · 2013-03-18T09:19:57Z

@mtbc #897 could be helpful in cases like this.

joshmoore · 2013-03-19T09:16:41Z

Label didn't get removed for today. @mtbc, should it for tomorrow? Would you like to wait on changes from @bpindelski?

bpindelski · 2013-03-19T09:26:20Z

@joshmoore I think all the changes @mtbc needs are in #897. That will allow for removing the dependancy on ArrayUtils and Arrays. I'm not planning on any other major changes wrt. checksums on dev_4_4 besides #897.

mtbc · 2013-03-19T09:26:54Z

Yeah, remove it today and I'll push another commit later that is based on #897 so that the revised version gets built tomorrow morning.

mtbc · 2013-03-19T12:30:14Z

Should there be a new exception class for checksum mismatches that has the client and server checksums and file path as properties? (Extending which class?) Or is what's here good enough for now?

bpindelski · 2013-03-19T12:42:54Z

@mtbc On develop we currently do https://github.com/openmicroscopy/openmicroscopy/blob/develop/components/blitz/src/ome/services/blitz/repo/ManagedImportProcessI.java#L261. Would be interesting to see how that can be handled on dev_4_4.

mtbc · 2013-03-19T16:14:12Z

Further commit will be tomorrow, not today. (I'm not even going to get finished with permissions testing today.)

mtbc · 2013-03-20T12:51:45Z

Okay, should be ready for testing again.

joshmoore · 2013-03-20T15:55:58Z

Unlabeled.

joshmoore · 2013-03-21T10:20:29Z

One further location that could use your inspection:

https://github.com/openmicroscopy/openmicroscopy/blob/develop/components/blitz/src/omero/client.java#L1029

(in fact, any location that calls write() on a RawFileStore probably falls into that category, but that may be beyond the scope at this point)

mtbc · 2013-03-21T10:26:13Z

I think the finally still executes despite the return but, yes, I might even try logging the saves and closes to spot where the leaks might be.

joshmoore · 2013-03-21T10:27:12Z

Basic archived import's certainly still work fine:

hudson@howe:~/OMERO-CURRENT$ echo "hi" >> pr890.fake
hudson@howe:~/OMERO-CURRENT$ bin/omero -s user-1@localhost import -a pr890.fake
omero=# select id, name, sha1 from originalfile order by id desc limit 11;
  id   |                               name                                |                   sha1
-------+-------------------------------------------------------------------+------------------------------------------
 41451 | pr890.fake                                                        | 55ca6286e3e4f4fba5d0448333fa99fc5a404a73
(1 row)

omero=# \q
hudson@howe:~/OMERO-CURRENT$ shasum pr890.fake
55ca6286e3e4f4fba5d0448333fa99fc5a404a73  pr890.fake

Though I'm still pondering who to make the checksum check fail.

mtbc · 2013-03-21T10:32:04Z

If it doesn't unduly invalidate the review, when the server or client hasher is constructed, put an extra byte into it so it'll screw up. (-:

joshmoore · 2013-03-21T10:36:26Z

OmeroPy/src/omero/util/populate_metadata.py also does not call close() on the TablePrx.

joshmoore · 2013-03-21T10:38:24Z

@mtbc: this PR all looks healthy and happy even if there are various things outside of the PR which are less so. Let me know if you would like to handle those in a separate PR or here.

mtbc · 2013-03-21T10:40:48Z

I'd like to fix populate_metadata.py here and RawFileStore leakage separately.

joshmoore · 2013-03-21T10:53:52Z

Re: an extra byte -- :). Maybe my own hasher subclass that just always return "".
populate_metadata.py v. RawFileStore: check, but not I didn't see a rawFileStore.close(). If I'm just overlooking that, then perhaps there's no problem.

joshmoore · 2013-03-21T11:03:36Z

Faked a failure. 👍

hudson@howe:~/OMERO-CURRENT/pr890$ export CLASSPATH=`find ../lib/client/ -name "*.jar"  | perl -pe 's/\s+/:/g'`
hudson@howe:~/OMERO-CURRENT/pr890$ JAVA_OPTS="-cp .:$CLASSPATH" ../bin/omero -s user-1@localhost import -a pr890.fake
...
2013-03-21 11:01:28,426 3966       [      main] INFO         ome.formats.importer.ImportLibrary  - Reader is not of HCS domain, use metafile: true
FAKE CHECKSUM
2013-03-21 11:01:28,752 4292       [      main] ERROR     ome.formats.importer.cli.ErrorHandler  - FILE_EXCEPTION: /home/hudson/hudson/workspace/OMERO-merge-stable/src/dist/pr890/pr890.fake
java.io.IOException: file checksum mismatch on upload: /home/hudson/hudson/workspace/OMERO-merge-stable/src/dist/pr890/pr890.fake (client has fake, server has 55ca6286e3e4f4fba5d0448333fa99fc5a404a73)
    at ome.formats.OMEROMetadataStoreClient.writeFilesToFileStore(OMEROMetadataStoreClient.java:1859)
    at ome.formats.importer.ImportLibrary.importImage(ImportLibrary.java:694)
    at ome.formats.importer.ImportLibrary.importCandidates(ImportLibrary.java:298)
    at ome.formats.importer.cli.CommandLineImporter.start(CommandLineImporter.java:143)
    at ome.formats.importer.cli.CommandLineImporter.main(CommandLineImporter.java:462)
2013-03-21 11:01:28,824 4364       [      main] ERROR     ome.formats.importer.cli.ErrorHandler  - INTERNAL_EXCEPTION: /home/hudson/hudson/workspace/OMERO-merge-stable/src/dist/pr890/pr890.fake
java.lang.RuntimeException: java.io.IOException: file checksum mismatch on upload: /home/hudson/hudson/workspace/OMERO-merge-stable/src/dist/pr890/pr890.fake (client has fake, server has 55ca6286e3e4f4fba5d0448333fa99fc5a404a73)
    at ome.formats.importer.ImportLibrary.importCandidates(ImportLibrary.java:301)
    at ome.formats.importer.cli.CommandLineImporter.start(CommandLineImporter.java:143)
    at ome.formats.importer.cli.CommandLineImporter.main(CommandLineImporter.java:462)
Caused by: java.io.IOException: file checksum mismatch on upload: /home/hudson/hudson/workspace/OMERO-merge-stable/src/dist/pr890/pr890.fake (client has fake, server has 55ca6286e3e4f4fba5d0448333fa99fc5a404a73)
    at ome.formats.OMEROMetadataStoreClient.writeFilesToFileStore(OMEROMetadataStoreClient.java:1859)
    at ome.formats.importer.ImportLibrary.importImage(ImportLibrary.java:694)
    at ome.formats.importer.ImportLibrary.importCandidates(ImportLibrary.java:298)
    ... 2 more

java.lang.RuntimeException: java.io.IOException: file checksum mismatch on upload: /home/hudson/hudson/workspace/OMERO-merge-stable/src/dist/pr890/pr890.fake (client has fake, server has 55ca6286e3e4f4fba5d0448333fa99fc5a404a73)
    at ome.formats.importer.ImportLibrary.importCandidates(ImportLibrary.java:301)
    at ome.formats.importer.cli.CommandLineImporter.start(CommandLineImporter.java:143)
    at ome.formats.importer.cli.CommandLineImporter.main(CommandLineImporter.java:462)
Caused by: java.io.IOException: file checksum mismatch on upload: /home/hudson/hudson/workspace/OMERO-merge-stable/src/dist/pr890/pr890.fake (client has fake, server has 55ca6286e3e4f4fba5d0448333fa99fc5a404a73)
    at ome.formats.OMEROMetadataStoreClient.writeFilesToFileStore(OMEROMetadataStoreClient.java:1859)
    at ome.formats.importer.ImportLibrary.importImage(ImportLibrary.java:694)
    at ome.formats.importer.ImportLibrary.importCandidates(ImportLibrary.java:298)
    ... 2 more
2013-03-21 11:01:28,824 4364       [      main] INFO         ome.formats.importer.ImportLibrary  - Exiting on error

hudson@howe:~/OMERO-CURRENT/pr890$ cat ome/util/checksum/SHA1ChecksumProviderImpl.
cat: ome/util/checksum/SHA1ChecksumProviderImpl.: No such file or directory
hudson@howe:~/OMERO-CURRENT/pr890$ cat ome/util/checksum/SHA1ChecksumProviderImpl.java
package ome.util.checksum;
import com.google.common.hash.Hashing;

public final class SHA1ChecksumProviderImpl extends AbstractChecksumProvider {

    public SHA1ChecksumProviderImpl() {
        super(Hashing.sha1());
        System.out.println("FAKE CHECKSUM");
    }

    @Override
    public String checksumAsString() {
        return "fake";
    }

}

joshmoore · 2013-03-21T11:04:32Z

We'll likely need to decide on the usability of this at some point, since this leaves garbage on the server that the user may want to delete immediately. /cc @jburel @gusferguson

mtbc · 2013-03-21T11:18:25Z

True, though they may also want it left around for a while first so the sysadmin can investigate the corruption.

gusferguson · 2013-03-21T11:20:18Z

Will require some form of notification - so an option in that to delete or retain?

jburel · 2013-03-21T12:40:54Z

if we have a checksum mismatch, I was planning to add a delete control. I used to have code that will automatically delete the image if an error occurred.
For multi-images file, I discuss with @bpindelski:

Green tick if everything okay
Green tick with red cross if one file or more did not work
Red cross if failure
For point 2,3 delete option will be available.

mtbc · 2013-03-21T14:32:06Z

This seems to work for me, too. The rest of the resource leakage cleanup I'll leave for https://trac.openmicroscopy.org.uk/ome/ticket/10565

joshmoore · 2013-03-22T08:07:42Z

@jburel: do you have that ticketed elsewhere?

jburel · 2013-03-22T08:17:37Z

No I did not have the time to do it. I will do that today

jburel · 2013-03-24T20:10:53Z

ticket created: https://trac.openmicroscopy.org.uk/ome/ticket/10580

joshmoore · 2013-03-25T08:15:28Z

Thanks all.

Increase checksum noting and matching for imports.

mtbc reviewed Mar 15, 2013
View reviewed changes

mtbc added 4 commits March 20, 2013 12:27

Increase checksum noting and matching for imports.

2c22fd1

Fix API usage to Apache Commons, not Java 1.6.

b079190

Use putBytes from PR 897.

d4d9dbc

Complete ChecksumProvider Javadoc.

71c3755

Close new table in populating metadata.

e9d13b6

Add Guava JAR to Insight build.

9e72779

bpindelski mentioned this pull request Mar 21, 2013

Rebase of checksum work onto develop #905

Merged

joshmoore added a commit that referenced this pull request Mar 25, 2013

Merge pull request #890 from mtbc/trac-2581

44087d0

Increase checksum noting and matching for imports.

joshmoore merged commit 44087d0 into ome:dev_4_4 Mar 25, 2013

mtbc deleted the trac-2581 branch March 25, 2013 08:23

mtbc mentioned this pull request Mar 25, 2013

Increase checksum noting and matching for imports. (rebase) #947

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Increase checksum noting and matching for imports. #890

Increase checksum noting and matching for imports. #890

mtbc commented Mar 15, 2013

mtbc Mar 15, 2013

joshmoore commented Mar 18, 2013

mtbc commented Mar 18, 2013

bpindelski commented Mar 18, 2013

joshmoore commented Mar 19, 2013

bpindelski commented Mar 19, 2013

mtbc commented Mar 19, 2013

mtbc commented Mar 19, 2013

bpindelski commented Mar 19, 2013

mtbc commented Mar 19, 2013

mtbc commented Mar 20, 2013

joshmoore commented Mar 20, 2013

joshmoore commented Mar 21, 2013

mtbc commented Mar 21, 2013

joshmoore commented Mar 21, 2013

mtbc commented Mar 21, 2013

joshmoore commented Mar 21, 2013

joshmoore commented Mar 21, 2013

mtbc commented Mar 21, 2013

joshmoore commented Mar 21, 2013

joshmoore commented Mar 21, 2013

joshmoore commented Mar 21, 2013

mtbc commented Mar 21, 2013

gusferguson commented Mar 21, 2013

jburel commented Mar 21, 2013

mtbc commented Mar 21, 2013

joshmoore commented Mar 22, 2013

jburel commented Mar 22, 2013

jburel commented Mar 24, 2013

joshmoore commented Mar 25, 2013

Increase checksum noting and matching for imports. #890

Increase checksum noting and matching for imports. #890

Conversation

mtbc commented Mar 15, 2013

mtbc Mar 15, 2013

Choose a reason for hiding this comment

joshmoore commented Mar 18, 2013

mtbc commented Mar 18, 2013

bpindelski commented Mar 18, 2013

joshmoore commented Mar 19, 2013

bpindelski commented Mar 19, 2013

mtbc commented Mar 19, 2013

mtbc commented Mar 19, 2013

bpindelski commented Mar 19, 2013

mtbc commented Mar 19, 2013

mtbc commented Mar 20, 2013

joshmoore commented Mar 20, 2013

joshmoore commented Mar 21, 2013

mtbc commented Mar 21, 2013

joshmoore commented Mar 21, 2013

mtbc commented Mar 21, 2013

joshmoore commented Mar 21, 2013

joshmoore commented Mar 21, 2013

mtbc commented Mar 21, 2013

joshmoore commented Mar 21, 2013

joshmoore commented Mar 21, 2013

joshmoore commented Mar 21, 2013

mtbc commented Mar 21, 2013

gusferguson commented Mar 21, 2013

jburel commented Mar 21, 2013

mtbc commented Mar 21, 2013

joshmoore commented Mar 22, 2013

jburel commented Mar 22, 2013

jburel commented Mar 24, 2013

joshmoore commented Mar 25, 2013