On 2024-12-27 01:07, Shailesh Vashishth wrote:
I am trying to do the following ToDo in store_client.cc.
// TODO: Convert store_client into AsyncJob; make this call asynchronous
I do not recommend working on that to-do:
1. It is too complex, requiring good understanding of AsyncJob design
that may be difficult to obtain through reading documentation alone.
2. It may be premature because we probably should "Merge store_client
into StoreClient" first (or at the same time), as documented in another
TODO in StoreClient.h.
3. The change requires serious testing that would be difficult to
perform and nearly impossible to validate via review.
storeClientCopy2(this, sc);
Please help me understand why we need to make this call asynchronous?
The relevant StoreEntry::invokeHandlers() code is best interpreted as
the following pseudo-code loop:
for (const auto &client: entry->store_clients)
client->do_something_complex_with(entry);
In other words, the code is contacting individual store_client tasks in
a loop while asking each task to perform complex actions with numerous
side effects, including cases where an action of one task affects
another task (and/or the looping/entry object itself!).
This is essentially spaghetti code in disguise, where (very indirect!)
recursive function calls and same-object recursive "visits" have
replaced classic "goto" statements:
https://en.wikipedia.org/wiki/Spaghetti_code
Experience shows that humans are terrible at writing and especially
maintaining correct spaghetti code. This specific invokeHandlers() loop
has caused many costly problems! Not all of them have been solved IIRC.
Asynchronous calls untangle this mess because they prevent recursive
calls and revisits. They have their own cons, of course, but they are a
part of a much better overall design.
HTH,
Alex.
_______________________________________________
squid-dev mailing list
squid-dev@lists.squid-cache.org
https://lists.squid-cache.org/listinfo/squid-dev
