indexer: reuse one discovery client per URL (#197)

[0g-peterzhb]

Fixes #197. Report finding 25 — where the reported fix turned out to be inert and the real problem was one layer over.

What the report proposed, and why it doesn't work

Finding 25 says NewZgsClient leaks its RPC client when the initial shard-config lookup fails, and suggests client.Close() before the error return. I implemented that, measured it, and it changes nothing:

Scenario, 50 iterations	Descriptors leaked
failed init, client never closed	100
failed init, with the suggested Close()	100
successful init, explicitly Close()d	100

Two per client, regardless of outcome or cleanup. Close() releases no connections for an HTTP client, and providers.Option offers no way to supply a shared one — so the error path was never the issue.

The real problem

getFileLocation builds a client per candidate URL, on every lookup that reaches discovery, inside a loop over locations × ports:

zgsClient, err := node.NewZgsClient(url, nil, defaultZgsClientOpt)
if err != nil { continue }
defer zgsClient.Close()

So descriptor use grew with traffic, not with the number of nodes. In a long-running indexer that's unbounded from ordinary requests. The defer inside the loop compounded it, holding every probed client until the enclosing call returned.

Change

Key clients by URL and keep them. The cost becomes proportional to the number of distinct nodes seen, which the network bounds, instead of to the number of lookups.

Deliberately no eviction. An evicted client's connections wouldn't be reclaimed either — Close doesn't do that — so an LRU would restore the unbounded growth rather than cap it, just more slowly. Close() on the cache now shuts down every cached client.

A URL whose shard-config lookup fails is not cached, so a transient failure doesn't poison it permanently; there's a test.

Testing

Test
..._ReusesOneClientPerURL	50 lookups for one URL return the same client with no descriptor growth. Fails before the fix
..._DistinctURLsGetDistinctClients	cost scales with nodes, not traffic
..._FailedClientIsNotCached	a failed init is retryable

Descriptor counts come from /proc/self/fd or /dev/fd, skipping where neither is enumerable.

go build, go vet, gofmt and go test -count=1 ./... (14 packages) all pass.

---

This change is 

[0g-peterzhb]

Closing - not worth it.

The measurement stands: two descriptors per ZgsClient, released neither by Close nor by anything else reachable from here, and discovery constructs one per candidate URL on every lookup that reaches it. But I never showed that adds up to an operational problem. Reaching discovery at all requires the trusted set not to cover the file's shards, and I have no measurement of how often that happens in practice - so the growth rate is unknown, and against a typical descriptor limit it may never matter.

The fix also carried its own cost I underweighted: clients cached for the process lifetime with deliberately no eviction, which trades a slow descriptor leak for holding a client per node ever seen. That is defensible only if the leak is real in practice, which is exactly what is unestablished.