Elektra's locking strategy while recording

[markus2330]

I'm curious: could you point me to those lock plugins? I just took a quick look and didn't find any.

_Originally posted by @atmaxinger in https://github.com/ElektraInitiative/libelektra/pull/4892#discussion_r1193456326_

[markus2330]

But I'm curious: could you point me to those lock plugins? I just took a quick look and didn't find any.

The semlock plugin which was removed, because it didn't work on NFS, in 61de4dc451478c43dab1d7a61dd956779b04fd9e

@kodebach wrote:

Recording is simply a new source of conflict errors that needs to be properly documented.

Yes, I agree. Ideally it wouldn't introduce a new source by simply writing the change-tracking journal before the kdbSet commit happens (and conflicts while writing the change-tracking journal are the same as writing to other config files). As this would probably a quite big change (and maybe even does not work if you need to write something during kdbGet?), having proper locking and properly documenting what that means sounds like the second best option.

Is it at least guaranteed that you don't get conflicts while recording? Having both conflicts and locks sounds like the worst of both worlds.

[atmaxinger]

writing the change-tracking journal before the kdbSet commit happens

We can do that, but that would mean a bit of refactoring and providing a mechanism of a "distributed transaction" between two KDB instances. (Easiest way is probably to allow a way to manually call commit after a kdbSet, and the kdbSet itself returns before it performs the commit if this is required). I think it's doable, but not in the first version.

On the other hand, if we don't care (or if it's incredibly unlikely) that the commit of the configuration data could fail, we can just write the journal data before that, that wouldn't be a problem. I'd just have to move the invocation of the record hook up a few lines. Then we could also get rid of the aborting lock, and just wait on it. Because the resolvers for configuration data already guarantee that no two processes can write the same keys.

Is it at least guaranteed that you don't get conflicts while recording?

It depends on what you mean by "conflict". We abort immediately if we can't get the lock, so it behaves the same way as if another process would write to the same configuration file. Applications already need to handle that. And if they use the high-level bindings that's already handled for them. If we get the lock then it's guaranteed that no conflicts can happen.

[kodebach]

writing the change-tracking journal before the kdbSet commit happens

Based on what I can see in #4892, I'd also say this would be very complicated to do. If we update the session and then commit the changes, we need a rollback procedure for the session in case the commit fails. Otherwise, we end up with changes in the session, which never actually happened.

Like atmaxinger says, we'd basically need a kind of "distributed transaction", so that we can do the commit phase of two kdbSet in one operation.

IMO the best option would still be append-only session recording. Every kdbSet generates a new (uniquely named) file in the session directory. This cannot cause conflicts, since we use unique names. Recording always works. However, this would also be bigger change, because now you need to merge all the files into a single diff when the recording session is read. (As opposed to merging on write as we do now.)

Is it at least guaranteed that you don't get conflicts while recording?

If we get the lock then it's guaranteed that no conflicts can happen.

To be clear: This is at the cost of permitting only on concurrent modification of the entire global KDB, no matter how unrelated the modifications are. Currently, conflicts only happen when two processes write to the same file. With the recording lock, even write to different files have to be done one after the other.

IMO it is an acceptable trade-off, but it has to be clearly documented. For short-lived recording sessions it's fine to limit the entire system to a single kdbSet at a time. But as an audit tool the current recording is effectively useless. It would be a huge bottleneck on a system with multiple users and many processes writing concurrently (a main use case for auditing). Not least because our locking system is effectively a spin-lock.

[markus2330]

We can do that, but that would mean a bit of refactoring and providing a mechanism of a "distributed transaction" between two KDB instances. (Easiest way is probably to allow a way to manually call commit after a kdbSet, and the kdbSet itself returns before it performs the commit if this is required). I think it's doable, but not in the first version.

I don't think "distributed transaction" would be necessary, we can simply fail if two kdbSet are executed at the same time? (Like we already do.)

On the other hand, if we don't care (or if it's incredibly unlikely) that the commit of the configuration data could fail,

The commit is a sequence of rename, which already could fail. And it happens, you notice by having left-over files that the resolver didn't move nor delete. But it is unlikely, it mainly happens on crashes or resolver bugs (which haven't been around for a few years now).

we can just write the journal data before that, that wouldn't be a problem.

Actually a journal could even improve the current situation, as then we could recover from failing commits. But to keep the status quo would be okay (that commits might fail and we simply keep it as unlikely as possible). I think the implementation of a journal is way out of scope for 1.0.

I'd just have to move the invocation of the record hook up a few lines. Then we could also get rid of the aborting lock, and just wait on it. Because the resolvers for configuration data already guarantee that no two processes can write the same keys.

Sounds like an easy fix. But this would we get rid of locks completely this way? As you know, the problem with locks is that they create an unpredictable long delay inside of kdbSet. This influences heavily in which situation users are allowed to use kdbSet.

Is it at least guaranteed that you don't get conflicts while recording?

It depends on what you mean by "conflict". We abort immediately if we can't get the lock, so it behaves the same way as if another process would write to the same configuration file. Applications already need to handle that. And if they use the high-level bindings that's already handled for them. If we get the lock then it's guaranteed that no conflicts can happen.

I mean that you get the lock, do everything fine for recording but overall kdbSet still fails for some other conflict (another kdbSet was executed in the meantime).

Imho, ideal would be that the session recording file gets prepared before commit, and during commit it simply gets moved like all the other configuration files.

[markus2330]

Btw. a decision would have been a big advantage here, we are discussing this way too late.

[kodebach]

I don't think "distributed transaction" would be necessary, we can simply fail if two kdbSet are executed at the same time?

Not sure, sounds like without the lock there would still be a possibility of having either (1) changes recorded in the session that were aborted or (2) permanently commit changes that are not recorded in the session.

Also the session is recorded via an entirely separate kdbGet/kdbSet cycle on a separate KDB * instance. The way I understood "distributed transaction" was to mean, a transaction that coordinates between multiple active KDB * instances.

The commit is a sequence of rename, which already could fail. [...]

True, but currently a (file-based) backend (using resolver) is an atomic unit. Either all changes within a backend are permanently committed or none are.

Imho, ideal would be that the session recording file gets prepared before commit, and during commit it simply gets moved like all the other configuration files.

With the current recording setup, everything shares a single backend for the recording session. So it's not that simple, two simultaneous kdbSet could both prepare a new session file and only when it comes to committing, one of them will notice that there is concurrent write. So you have to commit the session first, to avoid having to abort after the configuration is committed. But then you have committed the session, if the configuration commit now needs to abort, there are changes in the session that did not happen.

Btw. a decision would have been a big advantage here, we are discussing this way too late.

We had multiple decision PRs with long discussions. IIRC back then the you said, implementation details should be discussed later.

IMHO what we did here is absolutely the correct approach. The only way to get a good solution for something as complex as this is attempting a solution and iterating on it.

[github-actions[bot]]

I mark this stale as it did not have any activity for one year. I'll close it in two weeks if no further activity occurs. If you want it to be alive again, ping by writing a message here or create a new issue with the remainder of this issue. Thank you for your contributions :sparkling_heart:

[github-actions[bot]]

I closed this now because it has been inactive for more than one year. If I closed it by mistake, please do not hesitate to reopen it or create a new issue with the remainder of this issue. Thank you for your contributions :sparkling_heart: