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.

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

kdb spec-mount incompatibilities #2760

Open
kodebach opened this issue Jun 8, 2019 · 11 comments

Comments

4 participants
@kodebach
Copy link
Contributor

commented Jun 8, 2019

Steps to Reproduce the Problem

Assume that spec.ini contains:

[]
mountpoint = mymountpoint.ini

[mykey]
type = long
sudo kdb mount spec.ini spec/tests/hello ni
sudo kdb spec-mount /tests/hello

# test 1
kdb set /tests/hello/mykey test

# test 2
kdb set user/tests/hello/mykey test

# test 3
kdb set /tests/hello/mykey test

# test 4
echo "mykey = test" | kdb import /tests/hello/mykey ini

Expected Result

That all 4 tests fail, with an error message from the type plugin.

Actual Result

test 1 fails as expected, all others (including test 3 succeed).

Further Information

spec-mount creates a weird cascading mountpoint. I assume this is what breaks everything.

The import command actually also has a problem with the cascading key. It results in merge problems because the kdbGet with the cascading key returned the spec namespace as well, which is not part of the imported KeySet.

@markus2330

This comment has been minimized.

Copy link
Contributor

commented Jun 8, 2019

Thank you for reporting this problem! It is always important to get feedback about parts that are not working as expected. A special thanks for the detailed description.

test 2 is already reported in #2561

test 3 is a newly found issue. The problem here maybe is that Elektra detects that the key is not changed at all.

test 4 is also a newly found issue but not surprising as cascading import/export are not yet implemented. We should fail in such cases with a "not implemented error". @Piankero in which error category would you see a "not implemented"? Maybe interface error?

@Piankero

This comment has been minimized.

Copy link
Contributor

commented Jun 8, 2019

Yes, this fits perfectly into Interface errors.

@kodebach

This comment has been minimized.

Copy link
Contributor Author

commented Jun 8, 2019

test 2 is already reported in #2561

#2561 doesn't mention spec-mount, which is the cause of these problems here.

test 3 is a newly found issue. The problem here maybe is that Elektra detects that the key is not changed at all.

No the problem actually is spec-mount.

kdb spec-mount creates a cascading mountpoint (e.g. /tests/hello). If we use a cascading kdb set the keys we create are put into this mountpoint as far as I can tell. However, if we use a parent key with namespace (e.g. user/tests/hello/mykey) in kdb set the key is created in a mountpoint of that namespace (e.g. user, user/tests or user/tests/hello depending on what is mounted). During any following kdb set whether cascading or not the mountpoint lookup in kdbGet returns this namespace specific mountpoint instead of the cascading one, because that is where the key we provided is located. This however means that we don't actually retrieve the specification from the spec namespace, which is why the spec plugin doesn't copy the metadata. At least that is what I observed in a quick debugging session (spec received a KeySet with size 1).


Note: Caching might hide this problem as it would retrieve the whole KeySet. However, I found this bug while working on LCDproc which uses specload and that breaks caching AFAIK.

@markus2330

This comment has been minimized.

Copy link
Contributor

commented Jun 9, 2019

#2561 doesn't mention spec-mount, which is the cause of these problems here.

Are you sure? I get the same behavior in your 4 cases if I use:

kdb setmeta /tests/hello/mykey type long
kdb mount hello.ecf /tests/hello type

But very interesting is that: mmap caching seems to change the behavior of these tests.

  • With caching test 1 and 3 fail.
  • Without caching test 1-3 fail.

Btw. test 4 also destroys spec.ini (see #2762). Maybe related is #2761.

@mpranj Do you know what is going on here?

We definitely need more tests with the tools. Especially for caching. (All tests should run with and without caching).

@kodebach

This comment has been minimized.

Copy link
Contributor Author

commented Jun 9, 2019

Are you sure? I get the same behavior in your 4 cases if I use:

Then the problem likely isn't spec-mount itself, but its use of cascading mountpoints.

@markus2330

This comment has been minimized.

Copy link
Contributor

commented Jun 9, 2019

Could you please describe the problem (maybe in a different issue) once again?

The bugs above can imho explained by broken implementation in kdb-tools but are not a result of Elektra.

Elektra itself of course allows to write broken configs if used improperly, e.g. not getting spec keys, removing spec keys before doing kdbSet, ..; at least as long as we do not turn off non-cascading kdbGet/kdbSet it will be always possible.

Btw. there is already a proposal to make every kdbGet, kdbSet getting the full configuration (always using /). @mpranj had the idea when implementing mmap. (I wrote it now down in #1291).Then we could avoid such problems. Some test cases still fail with this changed behavior, so we most likely won't have this soon.

But imho it is also okay if the tools can circumvent the validation with a -f option. This makes sense because also validation might be wrong.

@mpranj

This comment has been minimized.

Copy link
Member

commented Jun 9, 2019

@mpranj Do you know what is going on here?

When there is a cache hit (kdbGet), no validation or any other plugins are called. It could be that the data was somehow cached, although the example does not look like it. Cascading keys or not make no real difference to the cache, except the cache file path.

If this validation should happen in kdbSet, I don't know. The cache did not change anything directly in kdbSet.

@markus2330

This comment has been minimized.

Copy link
Contributor

commented Jun 9, 2019

For me it seems like that cached spec keys suddenly appear in a non-cascading kdbGet that previously were not there, so something like:

kdbGet ("/tests") returns:

  • spec/tests/mykey
  • user/tests/mykey

then, a later kdbGet ("user/tests") returns the same keys, although this was not the case without caching. Without caching, kdbGet ("user/tests") would only return user keys and the spec plugin would only do a no-op.

If it is an improvement of behavior or not is to be discussed (see #1291).

@mpranj

This comment has been minimized.

Copy link
Member

commented Jun 9, 2019

kdbGet ("/tests") returns:

then, a later kdbGet ("user/tests") returns the same keys

If this really happens, then it was definitely not intended with my implementation of the cache. I always check that the parentKey is the same or below, which is not the case here. If so, it's definitely a bug and I need to check it.

@kodebach

This comment has been minimized.

Copy link
Contributor Author

commented Jun 9, 2019

But imho it is also okay if the tools can circumvent the validation with a -f option.

Of course, but one kdb set call (so changing the value not the metadata) should never disable validation for future kdb set calls. Even with a -f option.

When there is a cache hit (kdbGet), no validation or any other plugins are called.

As stated above, I originally encountered this problem while doing some stuff with LCDproc and specload, so as AFAIK caching was effectively disabled. When I thought of this I mentioned that caching might change the behaviour, because of this reasoning:

  • I observed that during the kdb set user/tests/hello/mykey test call, the spec plugin only receives a single key.
  • I determined that this is the case, because of the parent key used by kdb set (i.e. user/tests/hello/mykey vs /tests/hello/mykey).
  • AFAIK if there is a cache hit kdbGet returns a KeySet containing all the requested keys, but might return more keys.
  • If this cached KeySet were to contain the spec namespace as well (for example because a cascading KeySet was cached from the last call), then everything would work as expected, because the global spec plugin would receive the specification during kdbSet. (Note: spec wouldn't be called during kdbGet AFAIK, but that doesn't matter, the problems occur later on in the kdbSet call.)

Could you please describe the problem (maybe in a different issue) once again?

Like described above, the problem boils down to the fact that elektraSpecSet does not receive any spec keys and therefore won't copy the metadata over to the user key. The second part of the problem is that, the version with an explicit namespace writes to a different mountpoint (not the one created by spec-mount). Therefore on a following call (get or set) all validation is ignored, as it is not part of this mountpoint.

@markus2330

This comment has been minimized.

Copy link
Contributor

commented Jun 10, 2019

Of course, but one kdb set call (so changing the value not the metadata) should never disable validation for future kdb set calls. Even with a -f option.

Unfortunately it can: if the previous kdb set call removed the spec. And maybe exactly this happened in your case: you might have ran into #2762 or #2761. If it is not like this, we need a reproducible way to trigger it. With the lines above I never had the problem (with and without caching), see my post above which errors I got (3 always failed, except if I executed 3 after 4).

I observed that during the kdb set user/tests/hello/mykey test call, the spec plugin only receives a single key.

Yes, this is because of #2561. If you want validation, you currently need to use cascading keys.

AFAIK if there is a cache hit kdbGet returns a KeySet containing all the requested keys, but might return more keys.

Also without cache hits, kdbGet can return any number of keys. (It might even return all keys).

The second part of the problem is that, the version with an explicit namespace writes to a different mountpoint

I need to debunk some myths:

  • spec-mount does nothing than a cascading mount: its feature is "only" that it automatically selects the plugins from what is needed in the spec
  • cascading mounts is the same as a separate identical mountpoints for each namespace except spec
  • all ways of mounting can also be achieved with kdb set in system/elektra
  • Elektra has no persistent knowledge other than the key database itself. So if it is not a bug in Elektra, it is a problem in how KDB was manipulated. (The new error concept by @Piankero should actually demonstrate the possibilities of errors quite clear. Unfortunately, broken KDB is not separated from other installation errors.)

We should write this down somewhere in the docu. What is a good place? FAQ? Is someone reading the FAQ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.