Fix npe on AWS Storage Tier and treat the STANDARD tier status first #855
Conversation
…tier for labs that have not implemented proper storage tiering
|
Looks good. |
|
Fantastic, thanks Jim! Will we see a release |
|
Also, do you want me to open another PR to apply this patch to |
|
Hi @brainstorm yes well will do a 2.8.11 release for this. Possibly this week but more likely next. I have merged this to master, which involved resolving some conflicts, I think it is o.k. but if can review or test it that would be good. The commit id is 39658c6 (last commit). |
|
Thanks Jim! Did you really push that commit to |
Fix npe on AWS Storage Tier and treat the STANDARD tier status first # Conflicts: # build.gradle # src/main/java/org/broad/igv/util/AmazonUtils.java
|
Oh you're right, committed not pushed, its pushed now.
…On Wed, Oct 21, 2020 at 3:50 PM Roman Valls Guimera < ***@***.***> wrote:
Thanks Jim! Did you really push that commit to master? I can only see
"fix unit tests" one from 5 days ago right now :-S
—
You are receiving this because you modified the open/close state.
Reply to this email directly, view it on GitHub
<#855 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAHD2HAXM6VGDDGFCPWRIN3SL5QSFANCNFSM4SVSANNA>
.
|
|
Jim, just tested it on |
|
@brainstorm What platforms are you testing on? Something in the 2.8.11 release is causing IGV to hang in Windows 10. Not an issue on MacOS. Its not necessarily this PR, but there is very little else that differs. The coding changes here look innocuous, but there are a few AWS library updates. Anyway my question is whether or not you have tested this on Windows 10 before I jump into myself. |
|
I mostly test it on OSX since it's what most of the staff uses... I'll install it on windows and give it a go soon today to see what's going on, thanks for the heads up and your testing! |
|
@jrobinso Sorry, but I cannot reproduce :/ I just tested it on Windows 2019 (should be similar to Windows 10 I reckon?) and it seems to work just fine?
|
|
I did not know until just now that there was a Windows 2019, but it
definitely doesn't work on Windows 10. It does work on Windows 7.
However it's not necessarily the PR. I have a Windows 10 machine
available and I plan to set up a dev environment to dig into this today.
…On Mon, Nov 2, 2020 at 3:00 AM Roman Valls Guimera ***@***.***> wrote:
@jrobinso <https://github.com/jrobinso> Sorry, but I cannot reproduce :/
I just tested it on Windows 2019 (should be similar to Windows 10 I
reckon?) and it seems to work just fine?
[image: Skärmavbild 2020-11-02 kl 21 46 58]
<https://user-images.githubusercontent.com/175587/97860641-9c345380-1d56-11eb-9697-9ea963f915f6.png>
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#855 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAHD2HBQZBBD5TCZ3C4LVOTSN2GM7ANCNFSM4SVSANNA>
.
|
|
@brainstorm, I should clarify "not work". It is noticeably slower, and can appear to hang when loading bam files on Windows 10. There are no coding changes that could cause this, so I'm looking at the Maven updates. Do you have any idea what this library does? http-client-spi |
|
I just tested without that dependency and all seems to work fine, so it should be safe to remove... is it the culprit? Otherwise I'm thinking of sync/async behaviors with http clients now that there's better threading in IGV... perhaps this is related to the threading changes you were working on a short while ago? Here's what AWS SDK Java v2 says about http clients: https://docs.aws.amazon.com/sdk-for-java/v2/developer-guide/client-configuration-http.html I think we are just using synchronous. It's curious that just windows 10 is exhibiting this... different OS threading handling I (wild) guess? Can you pinpoint at which point is this slowdown happening?: Startup, S3Presign, reading the reads, etc... |
|
Yes its very curious, the coding changes between 2.8.10 and 2.8.11 are minimal. However there are a substantial number to changed jars in the final build, probably pulled in as AWS jar dependencies.
Your intuition about threading could well be the case, but what are you referring with re "... now that there's better threading ..."? Nothing's change in our code between 2.8.10. I don't understand the relevance of the link you referenced, we don't use ApacheHTTPClient and I don't recall working on this issue.
Since the dependency referenced apparently isn't needed are there others that could be removed?
|
I meant some sync/threading changes that went through on #833 (comment) ... most probably unrelated, but just trying to rewind commits on my memory that can be the source of the current issue.
IGV might not, but somewhere inside AWS Java v2 SDK it could, that's why I was asking more details about at which point do you see the slowdown... right after trying to load a file (could be S3 pre-signing), downloading the reads (could be threads issue), reading from a local BAM file (could be htsjdk BAM reader)? etc...
Those three remaining AWS deps on |
|
I need to break for a few hours. This should have nothing to do with S3 pre-signing, I'm reading from public buckets. AWS isn't even configured on my setup. Now I have a new challenge, I could reliably hang it earlier today now not at all. It hangs, when it hangs, as alignments are read. I say "hang", but the user who first reported this said it did load eventually, just very slowly. I don't think this has anything to do with the PR or new libraries, except perhaps exposing a bug waiting to happen. Anyway that's all for now, might have an update later this evening (or whatever it is for you). BTW one curious phenom, when it hangs the status message on the lower left of the status bar changes to "Iterating...". What's curious about that is I can't find anywhere in the code that sends such a message. So first thing in the next go at this will be to determine where that's coming from. |
|
I'm not really sure what the 2.8.11 problem was, but it wasn't this PR. Now released in 2.8.12. |
|
Thanks Jim! Intermittent bugs are the most annoying, we'll keep an eye on our Windows 10 users and see if they hit that issue. I'll relay that release announcement to my peers, much appreciated ;) |
Fixes observations made on #620 (comment) from two independent genomics labs.
This patch should also be cherry picked for next releases as well and the error unfortunately affects IGV versions ranging from 2.7.1 to 2.8.0 as pointed out by @welcomege.
Alternatively, I can also open a separate pull request against
master.