[ntuple] Fix race in cluster pool

[jblomer]

Fixes #16936

Also extends the RandomAccess unit test to increase the chance of hitting race conditions (x10 more random requests).

[github-actions]

Test Results

    19 files      19 suites   4d 12h 35m 15s ⏱️
 2 677 tests  2 676 ✅ 0 💤 1 ❌
48 946 runs  48 945 ✅ 0 💤 1 ❌

For more details on these failures, see this check.

Results for commit c118bff.

♻️ This comment has been updated with latest results.

[jblomer]

Still something strange on mac14. Could be a memory leak. Investigating.

[jblomer]

Doesn't seem to be a leak but just excessive memory consumption with the access pattern. That's a separate issue to address. It's most likely the page pool that stores, multiple times, the same uncompressed pages.

[hahnjo]

Can you maybe add some explanation to the last commit's message why fIsExpired is not needed (anymore)? Is it because with the removal of the background unzipping earlier this year, the loaded clusters are directly given back to the main thread?

[jblomer]

Can you maybe add some explanation to the last commit's message why fIsExpired is not needed (anymore)? Is it because with the removal of the background unzipping earlier this year, the loaded clusters are directly given back to the main thread?

Sure, and yes. Updated the commit message to

The fIsExpired flag was used to indicate to the I/O thread that a loaded
cluster is not needed anymore. This was useful at the time when the I/O
thread also took care of the parallel decompression. In this way, we could
avoid useless decompression of clusters.

However, now the parallel decompression is anyway triggered by the main
thread. The loop over fInFlightCluster in RClusterPool::GetCluster()
already avoids decompression if a cluster was expired.

Hence, the out-of-band signal from the main thread to the I/O thread is
not needed anymore.

[hahnjo]

LGTM