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

cache: problems with non-file-based storage plugins #2454

Open
mpranj opened this Issue Mar 2, 2019 · 3 comments

Comments

Projects
None yet
2 participants
@mpranj
Copy link
Member

mpranj commented Mar 2, 2019

@markus2330 as discussed in the last meeting, caching works with cascading keys too. Because of your comment about "/", I tested it and found it to be an exception.

The problem is that non-storage based backends like system/elektra/version or system/elektra/modules/list break the concept by always returning 1 (update needed), thus producing a cache miss.

I see two approaches to solve this:

  1. Simply cache those keys and ignore the "update needed" in that case. This approach might be problematic if plugins do weird stuff in their config/contract keyset, like putting in a timestamp or so.

  2. Omit the problematic keys from the cache and always retrieve them. The performance problem is that we always need to append them. The solution is somewhat related to solution 2 from #2269.

@mpranj mpranj added the question label Mar 2, 2019

@mpranj mpranj self-assigned this Mar 2, 2019

@mpranj mpranj referenced this issue Mar 2, 2019

Open

Global Cache #2270

1 of 8 tasks complete
@markus2330

This comment has been minimized.

Copy link
Contributor

markus2330 commented Mar 2, 2019

Thank you for reporting this problem!

Isn't this problem unrelated to cascading keys and simply a problem of that we need some way to properly implement and use "noresolver" (for the mentioned mountpoints but also for kdb mount-info and similar plugins?)?

The noresolver could work like: It gets all keys, and compares with the current keys. If the keys are the same, it returns 0 (cache hit). Otherwise, it returns 1 (update needed).

But actually, your idea (2.) to simply always append all keys again is maybe even better: then we do not need so complicated resolvers. And we wanted to use this behavior for command-line anyway. What is the performance penalty for doing so?

@mpranj

This comment has been minimized.

Copy link
Member Author

mpranj commented Mar 2, 2019

Isn't this problem unrelated to cascading keys [...]

You're right. In case of the cache, cascading keys just trigger the problem, but are not the cause.

What is the performance penalty for doing so?

Well, the optimal case for the mmap cache is of course not appending anything. In that case we only update pointers, which was very fast as shown with mmapstorage.
Since we have to fetch the new keys for both approaches, the penalty boils down to ksAppend/ksAppendKey.

Comparing the keysets sounds questionable to me. In case of a cache hit, we still have to compare two keysets. In case of a cache miss we have to correctly cut out the old keys and then append the new ones. It already does not sound very efficient.

Solution 2 more detailed:

In kdbGet() we would omit the problematic keys from the cache. The same is basically already done for the default split (the last split with user supplied keys, not from the backends). I propose to simply add a backendflag_t to mark such "problematic" backends, for easy detection.

After a cache hit we would then fetch only the problematic keys in elektraGetDoUpdate and simply append them.

@markus2330

This comment has been minimized.

Copy link
Contributor

markus2330 commented Mar 3, 2019

Well, the optimal case for the mmap cache is of course not appending anything.

When noresolver decides that there is no change, no ksAppend would be needed. The disadvantage is that you need to detect if there is no change. It is not clear for me if the detection or the appending is more expensive. For example, in a naive implementation of the ulimit plugin you would need to execute ulimit twice (first for the detection, then for appending; avoiding this would need changes in the framework).

In kdbGet() we would omit the problematic keys from the cache.

This would invalidate the OPMPHM. Maybe the ksAppend of existing keys is not so expensive after all: it only needs O(1) to locate where to insert and then O(1) to swap the key.

In any case: This problem does not need to be solved with your current (soon-to-come) PR.

@markus2330 markus2330 changed the title cache: problems with "/" cache: problems with non-file-based storage plugins Mar 3, 2019

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.