Enhance current get_result implementation

[venkatBala]

Current the get_result implementation keeps pinging the dispatcher's get_result endpoint incessantly until the result is ready to be retrieved. This causes a lot of CPU overhead and bottlenecks the system. The introduction of the timeout parameter may cause issues for long running workflows, since the request can simply timeout out before the workflow finishes resulting in HTTPTimeoutErrors

A more efficient solution might be to check if the result is in the database based on signals (multiprocessing queues, asyncio.subprocesses etc). Going forward a planning issue could be created in order to improve the current get_result implementation.

@FyzHsn @scottwn @wjcunningham7 @cjao

[scottwn]

The idea here is to get rid of while True

covalent/covalent_dispatcher/_service/app.py

Line 101 in ffadb62

while True:

What we want is a Python message queue on the server. When there are workflow updates, events are added to the queue. So instead of while True querying the DB for changes, we can query the queue asynchronously.

The responses sent by /result/<dispatch_id> would be the same. We may want to consider a longer retry-after or make that configurable.

We should also think about max_retries and timeout on the client side.

covalent/covalent/_results_manager/results_manager.py

Lines 155 to 159 in ffadb62

	adapter = HTTPAdapter(max_retries=5)
	http = requests.Session()
	http.mount("http://", adapter)
	url = "http://" + dispatcher + "/api/result/" + dispatch_id
	response = http.get(url, params={"wait": wait, "status_only": status_only}, timeout=2)

Should timeout be in get or HTTPAdapter or both? Either way, it should be an optional argument in get_result so the user can increase/decrease it if they want. Same with max_retries.

This whole block should be rewritten. time.sleep() should be removed if possible. Also we don't need to warn the user every time since the behavior is expected.

covalent/covalent/_results_manager/results_manager.py

Lines 72 to 78 in ffadb62

	except MissingLatticeRecordError as e:
	app_log.warning(
	f"Dispatch ID {dispatch_id} was not found in the database. Either the Dispatch ID is incorrect or wait a couple of seconds before trying again."
	)
	time.sleep(0.01)
	return get_result(dispatch_id, wait)

We want to make sure that /result/<dispatch_id> is returning 503 if the result is not ready to read yet, and 404 if it doesn't exist. The logic in app.py needs to be fixed so that the correct status code is returned in each case. Part of the issue is that these exceptions are never raised.

covalent/covalent_dispatcher/_service/app.py

Line 131 in ffadb62

except (FileNotFoundError, EOFError):

[scottwn]

Note that if the 503 and 404 are fixed, then the not found in the database warning is actually correct and the user will only be warned if the dispatch ID is really not found.

[cjao]

One problem with simply using a multiprocessing/multithreaded queue is that it would still block the web server from processing any additional requests until get_result returns. This would not be a problem with web servers that handle each connection in its own process or thread, but eventlet uses a single event loop using greenlets.

Update: we actually no longer use eventlet (which doesn't work with Python 3.10) but instead use the built-in Flask dev server (Werkzeug). Its concurrency model remains to be investigated...

[scottwn]

This work is being implemented in #35

[venkatBala]

@scottwn @wjcunningham7 @santoshkumarradha documenting few points from the team discussion we all had regarding the API changes with regards to ct.wait. I think the following API for get_result might be convenient

ct.get_result(dispatch_id=dispatch_id, wait=True, timeout=<xseconds>), where timeout defaults to ct.wait.EXTREME

[scottwn]

this is fine UX, but it would require some changes in implementation since there's not really a 1:1 relationship between time in seconds and retries. The client HTTP adapter accepts a number of retries, not seconds. We can make a rough conversion based on maximum retry-after, or we can do fancier math and get a closer conversion, but it won't be exact.

[scottwn]

I described the usage in the docstring 91eae30#diff-5694ed1aa0e5cf6e57cf19bb998f92485940c7df6209fa3b0d719a33fbb38802R161