fix: pop logs in log segment after cleaning up logs

[ion-elgreco]

Description

The description of the main changes of your pull request

Related Issue(s)

• closes cleanup_expired_logs_for doesn't update logSegment after removal #3062

[roeap]

We definitely should not be producing corrupt state and endanger querying an invalid snapshot.

However I hope we can consider a little bit the implications of some of these changes and the root cause we are looking at.

Need to dive a bit deeper, but right now I don't really understand how this can happen - if the metadata is part of the snapshot, it should still be valid? Or rather why would they get cleaned up if they are part of teh active snapshot, without any operations in-between that would cause the actions to no longer be part of a version?

Certain I am missing something, just don't know what this is.

More generally, log cleanup would usually be not on "the critical path" when it comes to cleanup it seems these would run even in completely separate processes? SO rather then trying to update the internal log, could we just consume the state and trigger a new replay? Or something along those lines?

In the "Pure" thinking snapshots are immutable - incremental updates are an optimization, but actually this just treats the current state as snapshot and replays new files on top of this.

Last but not least, I am hoping to integrate more with kernel and already did some promising experiments. Ideally we would keep the surface area of the snapshot minimal until we have some more clarity.

[ion-elgreco]

@roeap The snapshot is not valid anymore after you remove the logs from the object store. The logs don't exist, hence the snapshot should not reference them anymore.

However in its current state we still reference files in the log segment that are deleted from the object store, which causes vacuum to fail because it tries to read each file from the log segment

[roeap]

The snapshot is not valid anymore after you remove the logs from the object store.

I still donÄT understand how that situation arises :) - are we deleting data?

hence the snapshot should not reference them anymore.

could we just discard the snapshot then? i.e. have the function just consume the snapshot? It is a snapshot after all :) which can go invalid if the table is updated ...

[ion-elgreco]

The snapshot is not valid anymore after you remove the logs from the object store.

I still donÄT understand how that situation arises :) - are we deleting data?

Yes, cleanup_metadata removes the physical log files, and also post_commit_hook removes logs if the interval for it has been met.

So in two instances the Snapshot will become invalid. Only by forcefully reloading the table it will be fixed. The linked issue mentions the behaviour

hence the snapshot should not reference them anymore.

could we just discard the snapshot then? i.e. have the function just consume the snapshot? It is a snapshot after all :) which can go invalid if the table is updated ...

Replaying the log stream seems more compute intensive, then just popping the log files you know got deleted from the object store