oduwsdl / MemGator

A Memento Aggregator CLI and Server in Go
https://memgator.cs.odu.edu/api.html
MIT License
55 stars 11 forks source link

Timeout settings seem not to be working? #50

Closed ikreymer closed 8 years ago

ikreymer commented 8 years ago

I am running memgator with command: -V --arcs=/archives.json -t 3s -T 3s -r 3s server and when I run

time curl -I "localhost:1209/timenav/json/2007/http://google.com/"
real    0m15.244s

Seems like its still at the 15s default timeout?

ibnesayeed commented 8 years ago

I can reproduce it. Something is not behaving as expected. I will look at it over the weekend.

ikreymer commented 8 years ago

Any chance you could do it sooner? oldweb is experiencing heavy load and wanted to lower the timeout on the aggregator... I guess I could just look myself also..

ibnesayeed commented 8 years ago

Aggressively reducing the timeout will be dangerous as it will lose the recall. One quick solution would be to look at your logs and find out the archive that is hitting the timeout again and again. Just disable that archive for now and it should work fine. In the past I have seen an archive that performs OK on TimeMap requests, but takes forever on TimeGate.

ibnesayeed commented 8 years ago

I know why it is happening though. It is the response timeout that is holding the whole request this long, not the other two timeouts. When we make TimeMap request to individual archives, we expect 200 response code, so we use client.Do() call so that it handles any redirects internally and gives us the terminal response. In case of TimeGate, we expect a 302 (and optionally 200), which means we cannot let the default http client to follow redirects and would stick with the very first response we got. For this purpose we are using transport.RoundTrip() call. The client knows about all the three timeouts, but the transport only knows about two timeouts, not the response timeout. The fix would be to use a custom Dial function in the transport to pass the response timeout.

ikreymer commented 8 years ago

Are the command-line settings also not being set? I have all timeouts set to same value but it was still at 15s.. Trying hardcoding '5' instead of '15'.. Seems much easier then filtering list for each archive that may be slow..

ikreymer commented 8 years ago

Yeah, i can confirm that it does indeed work when i change the hardcoded default to '5'.. I did a build to netcapsule/memgator just with this change.. Assume that part is an easy fix.. Would be nice to be able to adjust timeout dynamically w/o restarting memgator in some way, but maybe for another time :)

Thanks for your help

ibnesayeed commented 8 years ago

Finally, I found the culprit. The values inside structs were assigned at the time of initialization and were not updated when the corresponding flag values (at pointers) were updated after parsing flags. Here is a code snippet to illustrate it.

I also, bumped up the default timeout values. @ikreymer, you should be fine using this, with appropriate flags, without hard coding. Please let me know if it still fails or breaks anything else.