exp/lighthorizon: Refactor archive interface and support parallel ledger downloads.

[Shaptic]

Note: This is a sizeable PR.

I recommend reviewing it commit by commit (link) rather than the whole thing at once to get a sense of the progression.

---

What

Major changes

This makes the following major changes:

• the data model (LedgerTransaction) just embeds an *ingest.LedgerTransaction directly; this is simpler until/if/when we decide to deviate from that
• Archive has been renamed Ingester and incorporates MetaArchive in the following way:
  • MetaArchive is now an interface
  • an Ingester extends MetaArchives with
    • PrepareRange (important for parallel downloads)
    • NewLedgerTransactionReader (to create a tx reader off of a ledger)
  • a basic liteIngester is just a MetaArchive + a network passphrase
  • a complex parallelIngester adds the ability to download ledgers in parallel (more below)
• GetTransactionParticipants and GetOperationParticipants are now standalone functions as there's no need for them to be tied to an instance
• searchAccountTransactions now prepares checkpoint ranges before iterating
• NewIngester creates a parallelIngester if the -parallel-downloads <count> parameter is appropriate
• Cursors can now Advance() more than once with a times parameter
• Fixes bugs in on-disk cache synchronization (this should be probably be a separate PR)

How parallel downloads work

The architecture for parallel download feeds (parallelIngester) follows:

• There exist workerCount perpetual worker routines that run forever, ready to take ledger sequences off of the queue and download them.
• When they complete a download they add it to the ledgerFeed (indexed by sequence) and notify along the signalChan.
• The consumer (i.e. GetLedger()) should wait on the signalChan and check the feed for the ledger it's looking for.
• The feed is carefully kept under a reader-writer lock.
• There's also a ledgerQueue so that consumers can know whether or not their requested ledger sequence is in the download queue. This lets certain ledgers be prioritized if they aren't in the queue already.

Why

Parallel downloads of ledgers have demonstrably decreased latency times, especially for requests that span large ledger ranges.

Closes #4468.

Known limitations

• The workers exist forever without a way to close them, but the web process should never close, meaning this is generally okay for an MVP.
• I'm not entirely sure the way the ledgerQueue, ledgerFeed, and GetLedger() interact is optimal.

[sreuland]

I noticed there's a ticket #4468 in the platform scrum for this effort, how about in that case then the PR can just state Closes <issue_no>rather than add PR to the project also, and then can move the #4468 to 'needs review' ? otherwise the board has two cards shown representing the same work.

[sreuland]

is it necessary to prune the feed entries per seqNum, what if for simplification, remove the write locking escalation from read lock, and just return the found feed state, and this just gets gc'd once when caller is done and its parallelIngester falls out of scope, could init the queue to empty at top of PrepareRange() to support caller doing multiple ranges on same instance of ParallelIngester?

This could also prevent caller from doing a re-entrant call to GetLedger with same ledgerSeq not falling into non-parallel ln 123

[Shaptic]

There already is support for multiple ranges! They will all get added to the queue. The only caveat is that each ledger will only be accessible one time (noteworthy in the case of overlapping ranges).

As for GC, isn't it the case that the parallelIngester will exist essentially throughout the duration of the program? It gets created once and passed around by reference everywhere. This would mean that the ledgerFeed would continue to grow and grow, essentially acting as an in-memory cache on top of the on-disk cache on top of the S3 ledger source. I'm not opposed to it necessarily (maybe we can actually use an LRU cache of uint32 -> downloadState instead of a map[uint32]downloadState), but I want to make sure that such a design doesn't just eventually OOM because I don't think the GC will ever kick in.

[sreuland]

yes, I'm wondering if you think it's worthwhile for simplification, to have instance of parallelIngester and queue/feed state scoped to per invocation of PrepareRange() such as a closure rather than as singleton (same instance per lifetime of program)? Ideally it seems this could avoid additional locking around shared state like here where wouldn't need to do the lock escalation from read up to write to purge, rather we know this queue instance will get gc'd as a whole when PrepareRange() invocation ends and any vars in it's closure lose reference.

Would that also safely allow parallel invocations of PrepareRange(), since each isolates on its own queue/feed, and they all converge into the single LRU cache which has concurrency already right?

[Shaptic]

I think I see what you're getting at. The model right now is something like:

• one parallelIngester is created on launch
• request comes in
• request gets processed with PrepareRange / GetLedger calls until its fulfilled
• another request comes in
• it uses the same instance to do PrepareRange / GetLedger calls
• etc. for every request in parallel

You note that this probably will cause high contention for the workers and the ledger feed (sync.Map still has to handle concurrency, after all). You propose that each request get its own dedicated worker pool, ledger feed, etc. in order to minimize contention. I think that's a great point, but I have some follow-up concerns:

1. Do you think each request getting --parallel-downloads workers dedicated to them will actually lead to too much saturation and churn on the network? This is my biggest concern. Imagine workerCount = 4 and we serve 100 requests per second. That's 400 concurrent downloads, open ports, etc. all to the same destination. Or should we keep the download worker pool global while the feeds/queues stay local as you suggest?
2. What about requests that share ledgers? Actually, answering this myself tells me that the on-disk cache should kick in as soon as one of the workers downloads the ledger in full.

[sreuland]

yes, the concurrent web requests end up time slicing over x workers, there might be a point of inflection on web response times where it's fast in low traffic but would slow down in higher traffic. But, I would avoid trying perf tuning this any further and instead get the functionality out as-is first to tune around if needed later. Already have a good algorithm here with some nice locking optimizations, so, should provide good starting point, thanks for considering the idea and discussion, I don't want to hold up the merge, thanks!

[sreuland]

how's the test coverage at this, I can't tell if it's used in other tests or mocked out. Maybe at some point a parallel_ingester_test.go will be worthwhile to assert unit testing of an instance and PrepareRange/GetLedger.

[Shaptic]

The test coverage is pretty non-existent besides my manual testing, unfortunately. I definitely want to mock out a parallel ingestion simulation but I'm worried about the sprint time crunch.

[sreuland]

ok, can circle back as tech debt, we should try to include the effort of writing tests in our point estimates during poker also as it's integral part of the feature, it's ok if the story doesn't close by eos due to test coverage, as it just reflects the accurate velocity of feature work.

[Shaptic]

yeah, that's on me for sure. I didn't factor in the challenge of testing the implementation during the poker (unfortunately it was also "in-progress" when we did poker so I had to point it myself)

[sreuland]

this new parallel download feature looks really good, I left some comments for consideration, lmk if you want to discuss, thanks!