dbus and rename plugin do not work together

[machinekoder]

There is a problem when the dbus and rename plugins are used at the same time. The dbus plugin seems not be aware of the renamed keys and notifies everyone with the raw key name. Furthermore it looks like the change detection does not work either.

$ kdb mount `pwd`/mkwrapper.ini system/machinekit/cnc ini type dbus rename tolower=0
$ kdb set system/machinekit/cnc/display/display test2

$ dbus-monitor --system type='signal',interface='org.libelektra',path='/org/libelektra/configuration'
ignal sender=org.freedesktop.DBus -> dest=:1.544 serial=2 path=/org/freedesktop/DBus; interface=org.freedesktop.DBus; member=NameAcquired
   string ":1.544"
signal sender=:1.545 -> dest=(null destination) serial=2 path=/org/libelektra/configuration; interface=org.libelektra; member=KeyChanged
   string "system/machinekit/cnc/AXIS_0"
signal sender=:1.545 -> dest=(null destination) serial=3 path=/org/libelektra/configuration; interface=org.libelektra; member=KeyChanged
   string "system/machinekit/cnc/AXIS_0/BACKLASH"
signal sender=:1.545 -> dest=(null destination) serial=4 path=/org/libelektra/configuration; interface=org.libelektra; member=KeyChanged
   string "system/machinekit/cnc/AXIS_0/FERROR"
signal sender=:1.545 -> dest=(null destination) serial=5 path=/org/libelektra/configuration; interface=org.libelektra; member=KeyChanged
   string "system/machinekit/cnc/AXIS_0/HOME"
signal sender=:1.545 -> dest=(null destination) serial=6 path=/org/libelektra/configuration; interface=org.libelektra; member=KeyChanged
   string "system/machinekit/cnc/AXIS_0/HOME_FINAL_VEL"
signal sender=:1.545 -> dest=(null destination) serial=7 path=/org/libelektra/configuration; interface=org.libelektra; member=KeyChanged
   string "system/machinekit/cnc/AXIS_0/HOME_IGNORE_LIMITS"
signal sender=:1.545 -> dest=(null destination) serial=8 path=/org/libelektra/configuration; interface=org.libelektra; member=KeyChanged
   string "system/machinekit/cnc/AXIS_0/HOME_IS_SHARED"
signal sender=:1.545 -> dest=(null destination) serial=9 path=/org/libelektra/configuration; interface=org.libelektra; member=KeyChanged
   string "system/machinekit/cnc/AXIS_0/HOME_LATCH_VEL"
signal sender=:1.545 -> dest=(null destination) serial=10 path=/org/libelektra/configuration; interface=org.libelektra; member=KeyChanged
   string "system/machinekit/cnc/AXIS_0/HOME_OFFSET"
signal sender=:1.545 -> dest=(null destination) serial=11 path=/org/libelektra/configuration; interface=org.libelektra; member=KeyChanged
   string "system/machinekit/cnc/AXIS_0/HOME_SEARCH_VEL"
....

mkwrapper.ini is here:
https://github.com/strahlex/mkwrapper-sim/blob/param/mkwrapper.ini

[markus2330]

When I mount it that way, I get:

system/elektra/mountpoints/system\/machinekit\/cnc/setplugins/#0#resolver
system/elektra/mountpoints/system\/machinekit\/cnc/setplugins/#1#type#type#
system/elektra/mountpoints/system\/machinekit\/cnc/setplugins/#2#rename
system/elektra/mountpoints/system\/machinekit\/cnc/setplugins/#5#ini
system/elektra/mountpoints/system\/machinekit\/cnc/setplugins/#6#sync#sync#
system/elektra/mountpoints/system\/machinekit\/cnc/setplugins/#7#resolver
system/elektra/mountpoints/system\/machinekit\/cnc/setplugins/#8#dbus

which gives dbus the keyset rename+INI got.

Possible solutions:

1. Add dbus also to the presetstorage (as very first, you should even give ordering constraint against rename so that it cannot be mount wrongly) and do change detection then (sending out notifications still in postcommit).
2. Global plugins (see in Global plugins #324) would give you all keys with sync flag correctly. This would also solve an additional problem (you might face later): you can implement a transaction across plugins (only in the global plugin you know that no other plugin will come after you).
3. Implement an unrename feature that in rename that places rename also in postcommit (in first position) and reverts what it has done before.

What do you think?

[machinekoder]

Solution 1 and 2 sound best. What is blocking the global plug-ins from being merged?

[markus2330]

The PR has some unresolved reviewer comments and needs a rebase.
@tom-wa Any plans on the timeline?

After the PR we need a kdb global-mount tool so that the global mountpoints actually can be used.

[markus2330]

Should be fixed now with global-mounts:

kdb mount `pwd`/machinekit.ini system/machinekit/cnc ini array= type rename tolower=0 # no notification needed here
kdb global-mount logchange # or dbus or both

Btw. INI plugin is not fully ready for the release yet, but should be done soon.

[markus2330]

Please confirm when you think it is ready.

[markus2330]

Btw. there are still notifications on any change (ADD+REM) for keys that are lowercase (wrong) in the configuration file. The renaming currently does not work bidirectional, so it does not ensure that new keys are written correctly. (See also discussion with @tom-wa #485)

[markus2330]

(Btw. I only tested with dump, as said, INI is on its way. With current INI there are still problems.)

[markus2330]

Still open, INI seems to change every key:

$ kdb set system/machinekit/cnc/pruconf/news fff
Set string to fff
changed key: system/machinekit/cnc/ABP
changed key: system/machinekit/cnc/ABP/SCALE
changed key: system/machinekit/cnc/ABP/STEPGEN_MAX_ACC
changed key: system/machinekit/cnc/ABP/STEPGEN_MAX_VEL

I think the most robust way is to make the change detection as early as possible so that it is not dependent on every plugin not making changing to keys.

[markus2330]

@waht is this now resolved as dbus does not notify every value individually anymore?

[stale]

I mark this issue 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 the issue by writing a message here or create a new issue with the remainder of this issue.
Thank you for your contributions 💖

[stale]

I closed this issue 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 💖

[markus2330]

@atmaxinger this is (hopefully) the last important issue that requires a decision in the context of change tracking.

Can you please write a decision with a summary of this issue. The problem also needs to be better clarified:

• What about plugins changing values back and forth (normalization, e.g. true -> 1, 1-> true)?
• What about plugins adding and removing metadata (like spec)

#955 is also part of this issue. Would it be possible to use iconv to rename keys?

[atmaxinger]

For me it looks like the main issue does not exist anymore.

My understanding is that these are two issues

1. The dbus plugin does not use the names as generated by the rename plugin.
2. The dbus plugin outputs all keys as changed. AFAIR this is a known issue that I also stumbled upon when I created the send-notification hook.

Number 2 is still an issue in certain cases, particularly here.

But Number 1 doesnt exist anymore, as dbus is now a notification-send hook plugin and gets executed after the poststorage phase.

This is the test setup that I have used:

kdb set system:/elektra/hook/notification/send/plugins/#0 dbus
kdb mount mkwrapper.ini user:/mkwrapper ini type rename get/case=tolower,set/case=toupper

At first, we register the dbus plugin to be used as a notification-send hook.
Then we mount mkwrapper.ini as user:/mkwrapper. The syntax for the rename plugin also changed since this issue was opened. In this example, we tell the rename plugin to transform the keys into lower case for use within Elektra, and to upper case for storage within the file.

kdb set user:/mkwrapper/cnc/display/display test1
kdb set user:/mkwrapper/cnc/display/display test2

> dbus-monitor --session type='signal',interface='org.libelektra'
signal time=1666947466.170558 sender=:1.343 -> destination=(null destination) serial=2 path=/org/libelektra/configuration; interface=org.libelektra; member=KeyAdded
   string "user:/mkwrapper/CNC/DISPLAY/DISPLAY"
signal time=1666947537.394764 sender=:1.344 -> destination=(null destination) serial=2 path=/org/libelektra/configuration; interface=org.libelektra; member=KeyChanged
   string "user:/mkwrapper/CNC/DISPLAY/DISPLAY"

mkwrapper.ini:

;Ni1
; Generated by the ni plugin using Elektra (see libelektra.org).

CNC/DISPLAY/DISPLAY = test2

[atmaxinger]

To expand why the change tracking of the dbus plugin does not work correctly in this case: The dbus plugin does not do a value-based change tracking, but instead uses keyNeedsSync to find out whether a key has been flagged as changed.

The way the rename plugin works, is that it deletes the old key and creates a new key with the renamed name. This causes keyNeedsSync to be true for every renamed key.

This is why the dbus plugin wrongly comes to the conclusion that the key changed. (And another reason WHY we need unified change tracking in Elektra, as proposed in #4554)

[atmaxinger]

@markus2330 there is actually a completely different new decision also in here: How should notification plugins deal with renamed keys?. If dbus uses the renamed keys, this breaks dbusrecv plugin as it will not find the keys with their renamed names in the KDB if get and set transformation are different!

[kodebach]

As @markus2330 said, it's not just about renamed keys. The decision is in general: Which version of the KDB should be used for notifications/change tracking?

Currently we use the version of the KDB that is present to applications. I think that is an easy option and probably the most consistent approach. Especially, since there could be plugins that add/remove keys based on other keys.

Another option is using the version that is returned by storage plugins. I think that would just lead to inconsistencies and confusion. It would also complicate things if you want to receive notifications inside applications. If people really need this, they should probably watch the storage file (or whatever storage is used) for changes and then call the storage plugin directly, when they detect a change.

With the simple phases approach we now have, I don't think there is any other option. We can't distinguish between plugins that change key names (or add/remove keys), plugins that only change values and plugins that don't change anything.

[markus2330]

For me it looks like the main issue does not exist anymore.

My understanding is that these are two issues

1. The dbus plugin does not use the names as generated by the rename plugin.
2. The dbus plugin outputs all keys as changed. AFAIR this is a known issue that I also stumbled upon when I created the send-notification hook.

Sorry, I didn't specify. This is a problem of such real-world issues, they often don't focus on some specific problem in a Elektra but to a problem a user had. I meant the problem "2.". But the problem is not specific to dbus but to how we detect changes.

And another reason WHY we need unified change tracking in Elektra, as proposed in #4554

Absolutely, I am so happy that you work on it! ❤️

How should notification plugins deal with renamed keys?

Excellent observation, this work will definitely earn the level of "master of science". Both problems are called in the scientific literature "feature-interaction problems".

Also as @kodebach said, there are other feature-interaction problems on general transformations (KeySets changing in the chain of plugins). Probably some restrictions of what plugins are allowed to do are necessary. I am not sure if renaming outside of storage plugins ever was a good idea but more about that when we have decisions.

[kodebach]

Probably some restrictions of what plugins are allowed to do are necessary.

I've been advocating for this for years now (e.g. #3497 (comment)) and was always shot down... Now that new-backend is done, this will be hard to implement without touching the fundamentals again.

One option I see is to define: "All key names are fixed after the storage phase. No keys can be added, renamed or deleted" (*) This would immediately prohibit a whole bunch of existing plugins. To solve that the backend plugin would need to be adapted to call plugins added/removing/renaming keys during the storage phase. That in turn means plugins need to declare in their contract what they actually do.

(*) needs slightly different wording to allow spec to do it's job. And also needs to be adapted for kdbSet.

[github-actions]

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 💖

[github-actions]

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 💖