[decision]: Changetracking (round 2)

[atmaxinger]

Basics

• Short descriptions of your changes are in the release notes
  (added as entry in doc/news/_preparation_next_release.md which
  contains _(my name)_)
  Please always add something to the release notes.
• Details of what you changed are in commit messages
  (first line should have module: short statement syntax)
• References to issues, e.g. close #X, are in the commit messages.
• The buildservers are happy. If not, fix in this order:
  • add a line in doc/news/_preparation_next_release.md
  • reformat the code with scripts/dev/reformat-all
  • make all unit tests pass
  • fix all memleaks
• The PR is rebased with current master.

Checklist

• I added unit tests for my code
• I fully described what my PR does in the documentation
  (not in the PR description)
• I fixed all affected documentation (see Documentation Guidelines)
• I fixed all affected decisions (see Decision Process)
• I added code comments, logging, and assertions as appropriate (see Coding Guidelines)
• I updated all meta data (e.g. README.md of plugins and METADATA.ini)
• I mentioned every code not directly written by me in reuse syntax

Review

• Documentation is introductory, concise, good to read and describes everything what the PR does
• Examples are well chosen and understandable
• Code is conforming to our Coding Guidelines
• APIs are conforming to our Design Guidelines
• Code is consistent to our Design Decisions

Labels

• Add the "work in progress" label if you do not want the PR to be reviewed yet.
• Add the "ready to merge" label if the basics are fulfilled and no further pushes are planned by you.

[atmaxinger]

I've mainly updated some alternatives with regards to full copy-on-write within Elektra.

One more thing that we need to discuss, is what we do with false positives - i.e. keys that are detected as changed even though practically they aren't. This can (and will) happen with plugins that do transformation of the key values.

As of now, I've added to the assumptions that false positives may be okay. We might be able to provide some advanced API that plugins can use to filter out false positives.

[markus2330]

One more thing that we need to discuss, is what we do with false positives - i.e. keys that are detected as changed even though practically they aren't. This can (and will) happen with plugins that do transformation of the key values.

Please elaborate (ideally within the decision, maybe in notes). If I change a value and then back it is obviously not the same reference (needs more memory) but a string-comparison still shows it is unchanged. Doesn't the comparison avoid any false positives?

[atmaxinger]

@markus2330 It is elaborated in the decision, under "Assumptions". It has nothing to do on how we compare values, but that plugins may change the values before writing to disk.

[markus2330]

Thx, I see! Couldn't this problem solved as well if we require users to call kdbSet after any changes in the KeySet and the transformation happens there? For this decision I would simply assume that transformations are done correctly and discuss how the transformations should work in the transformations decision. #4621

[markus2330]

Good to see progress here!

[atmaxinger]

Couldn't this problem solved as well if we require users to call kdbSet after any changes in the KeySet and the transformation happens there?

I don't see how this would solve anything. The big problem is that something other than the user can change the values.

This means that plugins using the change tracking API after the transformation step (e.g. notifications plugins and in particular the record plugin) will potentially get other change tracking results than plugins before the step.

From my POV, the "truest" change tracking results could be determined by:

• Storing a copy of the keys in kdbGet directly after the keys have been read from disk. Before any value transformations have been applied but after keyname transformations have been done
• Comparing it with the keys in kdbSet after everything is written to disk, but we somehow still have access to the keyname before the transformation.

This way, we would only detect if something truly changed. But this also means change tracking would be useless for all plugins other than notification hooks and the record hook. Those are the only plugins that are executed AFTER everything is done.

[kodebach]

From my POV, the "truest" change tracking results could be determined by:

That goes in the direction of what I suggested in #3497 a long time ago. But doing such a split on the libelektra-kdb level would be another (smaller) new-backend change.

I actually think, the "true" diff would be calculated between what is returned by kdbGet and what is passed to kdbSet. That's the data that is visible to applications, that is "Elektra's view of the world". The whole idea is that Elektra gives you a uniform view of the config, no matter what is actually stored, so that's what we should use.

This means that plugins using the change tracking API after the transformation step (e.g. notifications plugins and in particular the record plugin) will potentially get other change tracking results than plugins before the step.

I don't quite understand that statement. I'd assume the diff is calculated once in kdbSet and stored as long as kdbSet is running. If a plugin requests the diff via the API, it just gets this one diff, no matter when the plugin is executed.

We could of course also provide an API for calculating diffs (should be a separate library though). But that API should IMO take 2 KeySets as input and produce a diff. So if plugins used that API for some reason, they'd have to provide both KeySets. Then it is obvious why plugins get different results, if the have different KeySets

[kodebach]

I do agree with markus2330 that we should assume transformations are done correctly (whatever that may mean) in this decision. At least for now. If we find that there is no correct way to do them, then we may need to revisit this.

However, I also think false positives for values are not an issue at all. False negatives would be a much bigger issue, because that means we miss changes. If there is a use case where false positives must be avoided, we can just do a kind of re-check.

The change tracking plugin should have access to the current data. It can just check if the diff matches the current data and if not update the diff.

That said, I also believe that in your example the user using red (or in my example false) can simply be called incorrect. Then the reason for the false positive is simply that the user made an incorrect change. I say "incorrect", because the user should use the format returned by kdbGet. So if RED is returned then they should use RED and not red. In fact, you could even argue that the plugin that does the uppercasing should raise an error during kdbSet, if it sees a non-uppercase value.

[markus2330]

Please rerequest the review when I should have a look.

[atmaxinger]

I took the liberty and restructured this decision a bit. I have found that there are 3 largely independent things that we need to decide on here:

• Where to store changetracking data
• Where to implement changetracking
• How plugins and applications can interact with the changetracking mechanism.

[kodebach]

LGTM, but if you think the three parts are independent, it might make sense to split them into 3 decision files as well.

I'm not 100% sure it makes sense, since they are still quite related and share constraints, assumptions and most of the problem. But it may be easier to move forward, if we can decide the parts independently.

[atmaxinger]

Well, I don't think they are 100% independent. What I mean is that those are 3 areas we need to decide on, but it's still a single decision. Otherwise, I would have to repeat some stuff in each alternative (e.g., if the implementation is in a plugin, then it could still be possible to use backends->keys but also plugin-internal storage)

I just want to present some alternatives for each area, and then we pick the most suitable ones from each.

[atmaxinger]

jenkins build libelektra please

[atmaxinger]

@markus2330 could you please look over the decision?

[markus2330]

Definitely looks fine for in_discussion. Before it can be in solutions_clear please add how a plugin would provide "Solutions - Query".

[atmaxinger]

@markus2330 I have added an entry as to how the query mechanism could be implemented in a plugin. Do note, however, that I actually intended to only have the API within a library (i.e. libelektra-kdb). This API wouldn't care whether changetracking is implemented as a plugin or not.

[markus2330]

I think we can merge this but there are build errors.

[atmaxinger]

The build error is a timeout in npm install. I triggered a rerun of the failed job.

[atmaxinger]

jenkins build libelektra please

[atmaxinger]

@markus2330 @kodebach CI is green, we can merge now.