Re: aio: worker: Free SMGR objects when idle

Hi Nazir,

Andres, Shihao -- please check this is flawed.

You already proposed to use PGAIO_WORKER_SMGR_CLEANUP_INTERVAL and I think
that is better than PGAIO_WORKER_SMGR_CLEANUP_THRESHOLD from v3.

And I think that instead of trying to cover corner cases we have to design
a solution that has clear operation conditions (invariants).

Here I outline one solution, if you want to implement yourself.

Invariants (unless the process is sleeping):
Any open file was used at least once during the last two checkpoints.
Any open file was used at least once over the last OPEN_FILE_EXPIRE_COUNT
operations.

Actions:
- On a checkpoint destroy files that were not used since the previous
checkpoint
less destructive than destroy all.
- After an I/O closes the LRU file if it was not used during the last
OPEN_FILE_EXPIRE_COUNT

All you need to add is an I/O stamp on each entry.

This should keep the files that are actively used always open,
while setting a clear limit on how long a file can remain open.
io_count wraps safely,

Here I present a pseudo-code, suggesting the loop, but it could be called
from
the I/O functions themselves, integrating with the LRU update and protecting
not only I/O workers, but any process that uses this API.

Since we are only closing old files it is unlikely that we have to wait.

ckpt_io = 0
io_count = 0
loop (io) {
++io_count;
newest = get_file(io.rlocator);
newest.last_use = io_count;
lru_tounch_file(newest); // move to the top of the list.
oldest = lru.tail;
cleanup(cutoff);
}

cleanup(cutoff) {
// O(1) amortized, always check at most one surviving
// file per call, and the number destroyed files
// is less than the number of I/O.
// Closes at most one file except when crossing a checkpoint.

// entries not used over the last operations
cutoff = io_count - OPEN_FILE_EXPIRE_COUNT;
if FirstCallSinceLastCheckpoint() {
// destroys entries not used since the previous checkpoint.
cutoff = max(cutoff, ckpt_io - 1);
ckpt_io = io_count;
}

for(;;) {
oldest = lru.tail;
if (lru.tail == NULL) never
// wrap-safe comparison
if (lru.last_use - cutoff >= 0)
break; // always true for the last file used.
destroy(oldest);
}
}

I would remove BasicOpenFile from fd.c, that function is dangerous.

And of course remove the spin lock inside FirstCallSinceLastCheckpoint()

Regards,
Alexandre