Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

semantic of kdbget/kdbset #1363

Closed
BernhardDenner opened this issue Feb 22, 2017 · 6 comments
Closed

semantic of kdbget/kdbset #1363

BernhardDenner opened this issue Feb 22, 2017 · 6 comments
Labels
enhancement usability
Projects
lcdproc 1.0
Milestone
0.9.*

Comments

@BernhardDenner
Copy link
Collaborator

BernhardDenner commented Feb 22, 2017 •
edited

I've been wondering about the returned keySet values of kdbget for some time now, specifically why does the kdbget return more than I have requested?
I know, that kdbget will return the whole keySet the specific backend delivers, but from a users point of view this is a little bit strange and one could think "why do I get much more than I've requested?"

Moreover, multiple calls to kdbget do not have - from a users point of view - "expected" results.
For example, here the output of a simple test app, which performs several kdbget calls (each with a new KeySet):

## kdb.get 'user/sw/org/myapp'
## return value of kdb.get: 1, ks.size: 17
user/k1: 
user/myapp/#0/config1: value1
...
user/sw/org/myapp/#0/current: (binary) length: 0
user/sw/org/myapp/#0/testprofile: (binary) length: 0
user/sw/org/myapp/#0/testprofile/testkey: TEST
...
user/test/script/error/dump: important_unrecoverable_data
user/tests/ruby/exampleapp/#1/current/setting1: v2
...

## kdb.get 'user/myapp'
## return value of kdb.get: 0, ks.size: 0

In the first step the user gets much more keys than requested, also those for user/myapp (which he/she isn't interested now). In the second step he/she does not get any results as they where already delivered by the first kdbget call. This would be different if user/myapp is stored in a different backend. So a user has to keep in mind this technical organisation of the whole key space ("is everything in one backend or do I have to perform multiple kdbgets ?" ...)

As a user, I would intuitively expect something like:

kdb.get "user/sw/org/myapp"    # delivers all keys below and only below the specified path
kdb.get "user/sw/org/myapp"    # returns the exactly same result set
kdb.get "user/something/else"  # returns all keys below and only below "user/something/else"
# and for kdb.set
kdb.set keyset, "user/something/else"  # commits all keys below and only below "user/something/else"

... and hiding the technical implementation detail of backends.

Of course, this would also have impact on kdbset. The API docu describes this clearly enough:

These keys must be passed to calls of kdbSet(), otherwise they will be lost. This stems from the fact that the user has the only copy of the whole configuration and backends only write configuration that was passed to them.

Moreover, kdbset could ensure only to modify keys below the given parentKey, whereas the meaning of parentKey for kdbset would change from

its name is an hint which keys should be committed

to

its name ensures only keys below this path are committed

So the user will be sure only to modify keys he wants to.

Technically, this shouldn't be very hard to implement. The opened KDB handle could hold the whole KeySet delivered by the backend and kdbget returns a possible subset to this KeySet.

For users this change would lead to a more "intuitive" API.

Additionally I think we could than add something like

int kdbget(KDB* handle, Key* returned, Key* parentKey)
int kdbset(KDB* handle, Key* toSet, Key* parentKey)

for a shorthand of kdbget(); kslookup(); kdbset();

What do you think about this?
Or is it something for a more high-level API ?

PS: I considered to implement the Ruby-bindings in that way, but dismissed it, as it would be too big difference to the existing C-API (different behaviour of the same API functions could be dangerous).
@BernhardDenner
Copy link
Collaborator Author

maybe related to #1359 ???

@markus2330
Copy link
Contributor

Thank you for reporting!

This is the most-often reported issue, so we definitely need to fix it ;)
(e.g. #1258)

The plugin cachefilter implements the desired behavior, #690 proposes to have the new behaviour as default.

Technically, this shouldn't be very hard to implement.

Please test cachefilter (mount as global plugin, it should work now, thanks to @mpranj) if it does as you expect. It would also be great to have a new maintainer for cachefilter.

The kdbset feature to not change keys outside of the parentKey even though they are passed to kdbSet might be missing.

What do you think about this?

The convenience wrappers with char* instead of Key should be in a higher-level API. The parent Key is also used for error reporting, so it would not work like you suggest.

maybe related to #1359 ???

Only the last part about the convenience wrapper.

I considered to implement the Ruby-bindings in that way, but dismissed it

Correct decision! We should always fix issues at the root of the problem!

@markus2330 markus2330 added this to to discuss in lcdproc Mar 6, 2017
@markus2330 markus2330 moved this from to discuss to to implement in lcdproc Mar 16, 2017
@markus2330
Copy link
Contributor

The plugin cachefilter implements the desired behavior

I am afraid I have to qualify the statement. In Elektra it is also possible to have links to other keys which are looked up at run-time.

These keys can be outside of the parentKey. kdbGet should return them so that later lookups can find these keys.

We could:

  1. solve this issue in a higher-level API. (Like already done with elektraOpen with its argument)
  2. already resolve links using plugins during kdbGet. (Which would cause wrong lookups if the application modifies override keys or remove keys that have fallbacks)

What do you think?

@markus2330 markus2330 moved this from to implement to not needed in lcdproc Jul 6, 2017
@markus2330
Copy link
Contributor

(Not needed for lcdproc, thus the high-level API should abstract from that issue)

@markus2330 markus2330 added this to the 0.9.* milestone Oct 29, 2019
@markus2330 markus2330 added this to Semantics in 1.0 Oct 26, 2020
@stale
Copy link

stale bot commented Oct 28, 2020

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 stale bot added the stale label Oct 28, 2020
@markus2330
Copy link
Contributor

markus2330 commented Oct 29, 2020 •
edited

Unfortunately, we need to reject this issue:

  1. As it turns out, it is not easy to implement
  2. To have an internal extra KeySet is quite wasteful on memory. kdb.h is a low-level interface and should give users full control
  3. It is not a good design to have several caches, the cachefilter is somewhat contradicting to what the mmap cache does (especially it would create many performance problems the mmap cache is trying to solve)
  4. It does not fit with our decision to have a minimum number of global plugins, see doc/decisions/global_plugins.md

@stale stale bot removed the stale label Oct 29, 2020
@markus2330 markus2330 moved this from Semantics to Done in 1.0 Jul 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement usability
Projects
1.0
  
Done
lcdproc
not needed
Milestone
0.9.*
Development

No branches or pull requests
2 participants
@BernhardDenner @markus2330