NAVADMC / ADSM

A simulation of disease spread in livestock populations. Includes detection and containment simulation.
Other
10 stars 5 forks source link

Function Import Timeout #1018

Closed missyschoenbaum closed 3 years ago

missyschoenbaum commented 3 years ago

I'm getting a timeout when trying to load REL functions. It is Texas rel functions, which are big (about 3,500 records). This happened on MDL03 (the robust modeling PC, not my less robust desk PC).

504 Gateway Timeout

missyschoenbaum commented 3 years ago

More details on this. It actually loaded most or all of them, but didn't wrap up gracefully. I can document as needed. Next problem is that some PDF functions rely on RELs and yellow screen if they perceive function is missing. However, most of the PDF's load even in this case.

These are the cases where a PDF calls a histogram. The histogram is built as a REL, and then called as a PDF.

Is there are way (in the limited time we have) to skip if not found? If not, I will document.

ConradSelig commented 3 years ago

@missyschoenbaum Any chance you could add that REL function import to Google drive? I'm not sure how you got a 504 error, might be a syntax error in the file itself - but I can't say for sure without adding it.

There is currently no special treatment in the importer for the histogram PDF type - which is a likely reason that import is failing (regardless of if the corresponding REL function exists or not). I looked at ADSM itself to get a better idea of how histograms work and found that when creating a new PDF a drop-down "Graph" is created but none of the REL functions are listed! Is this a bug we need to fix? Or am I doing something wrong?

missyschoenbaum commented 3 years ago

@ConradSelig I just loaded up a PDF and a REL file exported from Texas. To test, I was just loading them into a copy of Sample. It might be that we need to just skip Graphs if that is possible.

I will go test other part of question and check back in.

ConradSelig commented 3 years ago

OK so looking into the code on how that "Graph" drop-down is looking for REL functions, here is what I found.

It is only looking for REL functions that match this naming scheme: "[ current PDF name] histogram data". So if I had a histogram called "histogram test", the only REL function that could show up would be one called "histogram test histogram data". One problem with this however, when you are creating a new PDF - that PDF doesn't have a name until you actually create it, just putting a name into the Name field doesn't count. This means that it can't actually even run this REL function search because the new PDF doesn't even have a name! Even worse, creating your new PDF function with a different type and then switching to the histogram test also does not appear to work, as there are still no REL functions showing up (even though I have a rel function that has the correct name). Maybe I don't actually know whats going on here? Or maybe the histogram is just busted and needs to be patched.

@missyschoenbaum @BryanHurst I'm sure one of you could provide some context here.

missyschoenbaum commented 3 years ago

@ConradSelig I think we just never caught this. Is it possible to allow on the PDF, that any rel can be valid?

missyschoenbaum commented 3 years ago

Also, do we need a new issue for this?

ConradSelig commented 3 years ago

Likely we can allow any REL, I'll look into it.

BryanHurst commented 3 years ago

On the timeout part of this, I'm concerned that the server is throwing that error. Other long running operations (like graceful startup) don't throw that error.

@ConradSelig are you able to replicate the timeout? Can we confirm if this is Nginx actually timing out, or that the view isn't returning a response because the import failed.

ConradSelig commented 3 years ago

I am working on replicating the error, but the import takes an extremely long time. I'm going to poke around in the import code for a minute and see if I can optimize it.

ConradSelig commented 3 years ago

While waiting for import I poked around at the code earlier and made some more discoveries about the histrogram-rel function link. Long story short I have come to believe that all REL functions should normally show up in that drop-down, so I'm not sure why none are. Looking into this.

ConradSelig commented 3 years ago

Unfortunately I have not been able to reproduce the import error. It could be Nginx that is the problem. My import generally takes 10-15 minutes.

@missyschoenbaum How long was this import taking for you (before it failed)?

missyschoenbaum commented 3 years ago

@ConradSelig It hasn't worked on this build, except on small scenarios. Could it be the repeating (duplicate) functions in the Texas import? As best I recall, it did take a while, so 10-15 minutes my best guess. That's why I don't understand why an insert of 3K rows should be taking so long, even though rels have to build the function and then build the points.

ConradSelig commented 3 years ago

When importing function it will always create as many functions as there are in the file. We handle duplicate function names by appending " - imported" to the end of the new function names.

I'm not surprised it takes a long time, because we are creating so many database entries the import operation is going to always be slow, even slower the more functions you are importing.

Still not sure what your error is about, especially since I'm not getting it even with the texas rel block.

ConradSelig commented 3 years ago

OK I found a fix for the missing functions bug. All of the created REL functions will be available for both the Histogram and Piecewise PDF functions.

As an additional note for clarity, unlike some REL selection drop-downs, this one does not have the "Add..." option that lets the user create a REL function on the fly. I tried to get around this for a bit, but because ADSM only allows for one right side fly-out at a time, you can only create one function at a time.

I don't think that this will cause any problems for users. Both the Histogram and Piecewise function forms are so fast to fill out that if they realize that they don't yet have the REL function they want practically no time will be lose. Under other circumstances we might decide to fight this, but with the end-of-contract tomorrow this is the best I can do.

ConradSelig commented 3 years ago

@missyschoenbaum Unfortunately barbecue I am unable to re-create your timeout issue, I don't have any way to determine what the problem might be. Unless you can provide some more information about it we are out of luck.

If that is the case then this ticket can be moved to testing. We can be glad we caught at least the one bug!

missyschoenbaum commented 3 years ago

@ConradSelig I missed the barbecue on the first round....You are correct

missyschoenbaum commented 3 years ago

Also, I agree they can create a rel before they decide to assign it.

BryanHurst commented 3 years ago

@ConradSelig is there any followup we want to do on this ticket?

BryanHurst commented 3 years ago

@missyschoenbaum is there anything you are hoping to follow up with on this ticket?

missyschoenbaum commented 3 years ago

No, we can close