Set pyramid thumb versions to -1 (Fix #12538) #3713

joshmoore · 2015-04-14T10:57:04Z

Thumbnails that are generated while pyramids are being
created (i.e. the clock symbol) should have a version of
-1 so that clients know to refresh.

There is currently no case where version == null is
used or even possible.

cc: @will-moore

Thumbnails that are generated while pyramids are being created (i.e. the clock symbol) should have a version of -1 so that clients know to refresh. There is currently no case where `version == null` is used or even possible.

joshmoore · 2015-04-14T11:05:31Z

Currently, after the pyramid is generated the thumbnail versions are not nulled.

will-moore · 2015-04-15T14:14:23Z

I think this is working fine. But for some reason I am still having trouble with thumbnail caching in web.
After importing the same big jpeg 6 times and pyramids created for them all, my thumbnail table looks like this:

  id  | permissions |  mimetype  | ref | sizex | sizey | version | creation_id | external_id | group_id | owner_id | update_id | pixels 
------+-------------+------------+-----+-------+-------+---------+-------------+-------------+----------+----------+-----------+--------
 8670 |         -40 | image/jpeg |     |    96 |    38 |      -1 |      445530 |             |        5 |        3 |    445530 |   7806
 8669 |         -40 | image/jpeg |     |    96 |    38 |       0 |      445514 |             |        5 |        3 |    445514 |   7806
 8668 |         -40 | image/jpeg |     |    96 |    38 |      -1 |      445514 |             |        5 |        3 |    445514 |   7806
 8667 |         -40 | image/jpeg |     |    96 |    38 |      -1 |      445493 |             |        5 |        3 |    445493 |   7805
 8666 |         -40 | image/jpeg |     |    96 |    38 |      -1 |      445487 |             |        5 |        3 |    445487 |   7805
 8665 |         -40 | image/jpeg |     |    96 |    38 |       0 |      445471 |             |        5 |        3 |    445471 |   7805
 8664 |         -40 | image/jpeg |     |    96 |    38 |      -1 |      445471 |             |        5 |        3 |    445471 |   7805
 8663 |         -40 | image/jpeg |     |    96 |    38 |      -1 |      445450 |             |        5 |        3 |    445450 |   7804
 8662 |         -40 | image/jpeg |     |    96 |    38 |      -1 |      445443 |             |        5 |        3 |    445443 |   7804
 8661 |         -40 | image/jpeg |     |    96 |    38 |       0 |      445426 |             |        5 |        3 |    445426 |   7804
 8660 |         -40 | image/jpeg |     |    96 |    38 |      -1 |      445426 |             |        5 |        3 |    445426 |   7804
 8659 |         -40 | image/jpeg |     |    96 |    38 |      -1 |      445396 |             |        5 |        3 |    445396 |   7803
 8658 |         -40 | image/jpeg |     |    96 |    38 |       0 |      445380 |             |        5 |        3 |    445380 |   7803
 8657 |         -40 | image/jpeg |     |    96 |    38 |       0 |      445380 |             |        5 |        3 |    445405 |   7803
 8656 |         -40 | image/jpeg |     |    96 |    38 |      -1 |      445358 |             |        5 |        3 |    445358 |   7802
 8655 |         -40 | image/jpeg |     |    96 |    38 |       0 |      445342 |             |        5 |        3 |    445342 |   7802
 8654 |         -40 | image/jpeg |     |    96 |    38 |      -1 |      445342 |             |        5 |        3 |    445342 |   7802
 8653 |         -40 | image/jpeg |     |    96 |    38 |      -1 |      445314 |             |        5 |        3 |    445314 |   7801
 8652 |         -40 | image/jpeg |     |    96 |    38 |       0 |      445294 |             |        5 |        3 |    445294 |   7801
 8651 |         -40 | image/jpeg |     |    96 |    38 |       0 |      445294 |             |        5 |        3 |    445319 |   7801

However, upon reloading the dataset in web (without browser refresh) all the versions are "-1" but I mostly have "timer" thumbnails still.

This means that at some point, with the version as "-1" the browser asked for a thumbnail and got a "timer" thumbnail, which got cached.

will-moore · 2015-04-15T14:48:15Z

I'm also struggling to know which version number to return for image.getThumbVersion(). I have been simply returning the highest version number for all the thumbnails for the image. But that fails with this PR since it will always return 0 instead of -1. So I have tried to return the version for the highest thumbnail.details.creationEvent.id but this fails because there are version 0 and version 1 for the same event id. Also, simply picking the version of the thumbnail with highest thumbnail ID seems to fail because the latest version may not be the most recently created thumbnail. E.g.

  id  | permissions |  mimetype  | ref | sizex | sizey | version | creation_id | external_id | group_id | owner_id | update_id | pixels 
------+-------------+------------+-----+-------+-------+---------+-------------+-------------+----------+----------+-----------+--------
 8667 |         -40 | image/jpeg |     |    96 |    38 |      -1 |      445493 |             |        5 |        3 |    445493 |   7805
 8666 |         -40 | image/jpeg |     |    96 |    38 |      -1 |      445487 |             |        5 |        3 |    445487 |   7805
 8665 |         -40 | image/jpeg |     |    96 |    38 |       0 |      445471 |             |        5 |        3 |    445471 |   7805
 8664 |         -40 | image/jpeg |     |    96 |    38 |       1 |      445471 |             |        5 |        3 |    445630 |   7805

joshmoore · 2015-04-15T16:18:04Z

I'll spend some more time on this tomorrow, @will-moore, but there's likely some state in the ThumbnailBean that I'm not taking into account. A refactoring would likely make it easier to detect what's going on, but that brings its own dangers. We'll see!

sbesson · 2015-04-16T15:24:31Z

Temporarily excluded to assess the cause of the pyramid generation hanging described in ome/bioformats#1725.

The retrieveThumbnail method was internally resetting the cached metadata object when inProgress was true. Now, we temporarily copy this value so that following saves don't create new objects.

joshmoore · 2015-04-21T14:42:22Z

Hopefully 0ac0f53 should fix @will-moore's issue.

joshmoore · 2015-04-21T20:21:46Z

Only one of the 2 tests is failing now (see 427). This is the less used method, so still some hope.

will-moore · 2015-04-22T09:15:36Z

@joshmoore This still isn't working for me on trout (haven't tried locally yet to dig any deeper).
When I tried this locally before, I was getting 3 thumbnails for each image (see table above), two of these were -1. Does this mean that a version with -1 exists before the pyramid has finished?

will-moore · 2015-04-22T09:23:03Z

Or maybe (as I asked above) there are times when my getThumbnailVersion() should be returning 0 instead of -1? E.g. Looking at the top 3 rows of the first table above (pixels = 7806), there is a version 0 and version -1 for the first update_id (445514), then there is another version -1 for a different update_id (445530). So, what should my logic for getThumbnailVersion() do?
E.g. Get the thumbs for the most recent updates (highest update_id) then choose the highest version number from these?

  id  | permissions |  mimetype  | ref | sizex | sizey | version | creation_id | external_id | group_id | owner_id | update_id | pixels 
------+-------------+------------+-----+-------+-------+---------+-------------+-------------+----------+----------+-----------+--------
 8670 |         -40 | image/jpeg |     |    96 |    38 |      -1 |      445530 |             |        5 |        3 |    445530 |   7806
 8669 |         -40 | image/jpeg |     |    96 |    38 |       0 |      445514 |             |        5 |        3 |    445514 |   7806
 8668 |         -40 | image/jpeg |     |    96 |    38 |      -1 |      445514 |             |        5 |        3 |    445514 |   7806

joshmoore · 2015-04-22T09:32:31Z

Can you may help me write a failing test? I'm running a bit blind at the moment.

will-moore · 2015-04-22T10:52:25Z

I don't know what the expected behaviour is yet, so don't know what to test.
Let's look at this scenario of importing a big image and generating pyramids.
NB: I haven't rebuilt locally with your changes from yesterday yet, so this is just about thumbnail version rather than "dupe thumbnails".

Below is my output from image.getThumbVersion() at different stages.

First lines are rows in the thumbnail table, printing out t.details.creationEvent.id and t.version, then there is a dict summary of max versions per eventId and then the VERSION that I'm returning.

Immediately after import has completed I see one event, with 2 thumb versions (0 and -1):

452924 0
452924 -1
{452924L: 0L}
VERSION 0

Next, I see this (I assume pyramid generation is completed now):

452942 -1
452924 0
452924 -1
{452924L: 0L, 452942L: -1L}
VERSION -1

At this point I load (and the browser caches) thumbnail?version=-1 but the thumbnail I am getting here still has the Clock.

Then there is some other event with version -1 and I'm now getting:

452948 -1
452942 -1
452924 0
452924 -1
{452924L: 0L, 452948L: -1L, 452942L: -1L}
VERSION -1

All this time, I still have thumb?version=-1 with the Clock cached on the browser, but if I open the thumb in a new tab and request:

/webclient/render_thumbnail/size/96/8351/?version=-1

I see the new image thumbnail. Generating this thumbnail updates the thumb table

452948 -1
452942 -1
452924 0
452924 0
{452924L: 0L, 452948L: -1L, 452942L: -1L}
VERSION -1

but my getThumbVersion() still gives me -1 since that is the version from the most recent event Id (452948).

Then I open the Preview tab and save a change to the rendering settings and request a new thumbnail. Now we have this:

452948 -1
452942 -1
452924 0
452924 1
{452924L: 1L, 452948L: -1L, 452942L: -1L}
VERSION -1

but I'm STILL getting -1 returned from my getThumbVersion() as above.

So, there's basically 2 problems above:

There is a thumbnail in the table above with version -1 BEFORE pyramid generation has completed (I think). In the scenario above, that doesn't cause a problem because my getThumbVersion logic picks version 0 since it has a more recent event ID.
However, once pyramid generation is complete, getThumbVersion returns -1 and I load the thumbnail it is still the Clock.
Now I'm going to rebuild with your latest changes to see if that issue is fixed. If so, we might be OK.

will-moore · 2015-04-22T11:41:35Z

Ok, trying the same thing again, with the rebuilt server.

Immediately after import and start of pyramid creation

2015-04-22 12:23:46,007 INFO  [                ome.io.nio.PixelsService] (2-thread-5) Pyramid creation for Pixels:7951 1/3072 (0%).

I see only a single event this time, with version -1. This is different from above where I saw versions 0 and -1.

# time 12:23:51
453102 -1
{453102L: -1L}
VERSION -1

Then, still during pyramid generation I see:

# 12:24:11
453120 -1
453102 -1
{453120L: -1L, 453102L: -1L}
VERSION -1

This is the same right after

2015-04-22 12:24:36,626 INFO  [                ome.io.nio.PixelsService] (2-thread-5) SUCCESS -- Pyramid created for pixels id:7951

I still see

# 12:24:39
453120 -1
453102 -1
{453120L: -1L, 453102L: -1L}
VERSION -1

Now, after I manually refresh thumb in browser (shows thumb image) I see

453120 -1
453102 0
{453120L: -1L, 453102L: 0L}
VERSION -1

So, the problem from above is that the thumbnail table has version: -1 thumbnails before Pyramid generation is complete.

will-moore · 2015-04-22T12:28:41Z

Not sure the best way to write a test for this.

joshmoore · 2015-04-22T13:06:31Z

So, the problem from above is that the thumbnail table has version: -1 thumbnails before Pyramid generation is complete.

Discussed this with @will-moore. This is exactly what I was expecting, and seems to match his local build behavior as described in #3713 (comment)

There was one misunderstand: the PixelData background process will not update the thumbnail version automatically. Only on the next call to ThumbnailBean will the version get bumped. @will-moore is going to implement a re-check of only those thumbnails which are -1.

cc: @jburel @dominikl -- in case something like this is useful.

More frequent closing of the ThumbnailBean (as Web does) led to more predictable failure. The issue seems again to be that for some code paths (`getThumbnail`), multiple tbs are being produced.

Several private methods have now been given the rewriteMetadata arg which decides whether or not the instance variable `thumbnailMetadata` is over- written when a `*Direct` method is called. This is a suboptimal fix since it leads to more complexity but until this class is reworked, this seems to be the safest.

joshmoore · 2015-04-23T09:41:31Z

Fix for unpredictable failures pushed. getThumbnail (unused) should be fixed now.

pwalczysko · 2015-04-24T09:42:00Z

The UI workflow from #3750 is going just fine and as expected, the bug is not there, the behaviour as expected. Looks good.

sbesson · 2015-04-24T10:08:27Z

Thanks @pwalczysko. Merging this backend PR. #3750 will need one last round of quick retesting after @will-moore's last commit.

Set pyramid thumb versions to -1 (Fix #12538)

atarkowska · 2017-02-13T16:38:34Z

This PR introduced inProgress but it missed that param in retrieveThumbnailSet, #5081 fixed this issue so we can retrieve thumbnails, although TestTree.test_marshal_images_thumb_version is failing

E         -   'thumbVersion': 0L}]
E         ?                   ^^
E         +   'thumbVersion': -1}]
E         ?                   ^^

why is expecting 0?

joshmoore · 2017-02-13T17:08:42Z

NB: 1011 from gh-5081 is https://github.com/openmicroscopy/openmicroscopy/pull/3713/files#diff-bae9d2b989f3f6e6de49048fa4dcd34dR979 here.

Set pyramid thumb versions to -1 (Fix #12538)

365b042

Thumbnails that are generated while pyramids are being created (i.e. the clock symbol) should have a version of -1 so that clients know to refresh. There is currently no case where `version == null` is used or even possible.

joshmoore force-pushed the 12538-tb-versions branch from e2761e9 to 365b042 Compare April 14, 2015 10:58

joshmoore added the develop label Apr 16, 2015

sbesson added the exclude label Apr 16, 2015

sbesson mentioned this pull request Apr 16, 2015

Zeiss CZI: fix series count for fused images ome/bioformats#1725

Merged

sbesson removed the exclude label Apr 16, 2015

joshmoore added 3 commits April 21, 2015 14:59

Update test to separate 'one' and 'set'

8665895

Further separate 'one' and 'set' methods

5aa9986

Handle dupe thumbnails (Fix #12538)

0ac0f53

The retrieveThumbnail method was internally resetting the cached metadata object when inProgress was true. Now, we temporarily copy this value so that following saves don't create new objects.

will-moore mentioned this pull request Apr 22, 2015

Pyramid generation thumbnails #3750

Merged

joshmoore added 2 commits April 23, 2015 11:33

Adjust test to more reliably fail

060eb24

More frequent closing of the ThumbnailBean (as Web does) led to more predictable failure. The issue seems again to be that for some code paths (`getThumbnail`), multiple tbs are being produced.

sbesson added a commit that referenced this pull request Apr 24, 2015

Merge pull request #3713 from joshmoore/12538-tb-versions

67fe98f

Set pyramid thumb versions to -1 (Fix #12538)

sbesson merged commit 67fe98f into ome:develop Apr 24, 2015

joshmoore deleted the 12538-tb-versions branch April 27, 2015 12:39

sbesson added this to the 5.1.1 milestone Apr 28, 2015

atarkowska mentioned this pull request Feb 13, 2017

fix getThumbnailsByLongestSideSet #5081

Merged

This was referenced Feb 13, 2017

Get thumbnails #5062

Merged

Load thumbnails #5112

Closed

atarkowska mentioned this pull request Mar 6, 2017

Get thumbnails (rebased onto develop) #5158

Closed

atarkowska mentioned this pull request Apr 4, 2017

Get thumbnails (rebased onto metadata53) #5233

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Set pyramid thumb versions to -1 (Fix #12538) #3713

Set pyramid thumb versions to -1 (Fix #12538) #3713

joshmoore commented Apr 14, 2015

joshmoore commented Apr 14, 2015

will-moore commented Apr 15, 2015

will-moore commented Apr 15, 2015

joshmoore commented Apr 15, 2015

sbesson commented Apr 16, 2015

joshmoore commented Apr 21, 2015

joshmoore commented Apr 21, 2015

will-moore commented Apr 22, 2015

will-moore commented Apr 22, 2015

joshmoore commented Apr 22, 2015

will-moore commented Apr 22, 2015

will-moore commented Apr 22, 2015

will-moore commented Apr 22, 2015

joshmoore commented Apr 22, 2015

joshmoore commented Apr 23, 2015

pwalczysko commented Apr 24, 2015

sbesson commented Apr 24, 2015

atarkowska commented Feb 13, 2017 •

edited

joshmoore commented Feb 13, 2017

Set pyramid thumb versions to -1 (Fix #12538) #3713

Set pyramid thumb versions to -1 (Fix #12538) #3713

Conversation

joshmoore commented Apr 14, 2015

joshmoore commented Apr 14, 2015

will-moore commented Apr 15, 2015

will-moore commented Apr 15, 2015

joshmoore commented Apr 15, 2015

sbesson commented Apr 16, 2015

joshmoore commented Apr 21, 2015

joshmoore commented Apr 21, 2015

will-moore commented Apr 22, 2015

will-moore commented Apr 22, 2015

joshmoore commented Apr 22, 2015

will-moore commented Apr 22, 2015

will-moore commented Apr 22, 2015

will-moore commented Apr 22, 2015

joshmoore commented Apr 22, 2015

joshmoore commented Apr 23, 2015

pwalczysko commented Apr 24, 2015

sbesson commented Apr 24, 2015

atarkowska commented Feb 13, 2017 • edited

joshmoore commented Feb 13, 2017

atarkowska commented Feb 13, 2017 •

edited