refactor/MTG-1254-json-downloader-service[mtg-144][mtg-526]

[kstepanovdev]

The draft of the architecture is here:
 

The core idea is to refactor the tasks column, removing redundant data from it and adding new data such as etag to reduce the database workload.
New workflow at first tries to get etag or last-modified-date if such fields are present in a dedicated asset and etag hasn't been changed it will not be written to the db.
Another small feature is leveraging tokio::select without an outer loop.

[armyhaylenko]

LRFBTM

[StanChe]

This is huge, thank you for the incredible work on it!
The PR should be refined though. Some major concerns around sql are in comments.
As an additional comment I'd like to highlight that we're no longer persisting the error we got during downloading, but it has some value, as we may distinguish some fixable errors (like a single/double quote as part of the uri), or some corner cases (local file path, dns errors, json directly embedded in the url)
Duplicating a comment from a ticket:
FYI, analyses of reasons of failure:

  AND tsk_error NOT LIKE '%source: TimedOut%'
  AND tsk_error NOT LIKE '%source: hyper::Error(Connect, ConnectError("dns error%' 
  AND tsk_error NOT LIKE '%source: hyper::Error(Connect, ConnectError("tcp connect%'
  AND tsk_error NOT LIKE '%source: hyper::Error(Connect, Ssl(Error { code: ErrorCode(%'
  AND tsk_error NOT LIKE '%source: TooManyRedirects%'
  AND tsk_error NOT LIKE '%source: hyper::Error(IncompleteMessage)%'
  AND tsk_error != 'Failed to parse URL: RelativeUrlWithoutBase'
  AND tsk_metadata_url NOT LIKE 'ipfs://%'
  AND tsk_metadata_url NOT LIKE 'ifs://%'
  AND tsk_metadata_url NOT LIKE 'data:application/json%'
  AND tsk_metadata_url NOT LIKE 'https:://%'
  AND tsk_metadata_url NOT LIKE 'ttps://%'
  AND tsk_metadata_url NOT LIKE 'hhttps://%'
  AND tsk_metadata_url NOT LIKE 'Ihttps://%'
  AND tsk_metadata_url NOT LIKE 'https:// %'
  AND tsk_metadata_url NOT LIKE 'file:%'
  AND tsk_metadata_url NOT LIKE '%"'
  limit 250;

That covers all but several hundred errors

[StanChe]

can/should we make the migration more "context-aware" for example, but keeping the success state, where it's present and keeping the last_modified_at to some old date? Or should this rather be run together with the JsonMigrator to restore the state in the DB? My main concern here is having to re-clarify (initial download flow) for the state of all 55+M urls while we're constantly receiving new ones as well

[StanChe]

Are we setting the next refresh/attempt time for immutable as well?

[StanChe]

Probably as a next ticket would be to make it more flexible, like data:Vec<(String,Duration)> to consider the cache validity, or any other logic we want to put for the next retry

[StanChe]

This seems to be an important comment here. You may get a deadlock without skipping locked. Here is how this may happen:

• this select selects for update 100 records, it acquires lock on the first 50 and tries to acquire it on the 51st;
• in a different process the synchronizer batch updates another 100 assets, 2 of which intersect, in the batch it has already updated 99 assets including the one this select tries to lock. Now it waits for the 100s which is being locked among other 50 here.
  Now we'll have a deadlock. I don't see this being addressed differently here.

[StanChe]

same deadlocking potential here