Move external auth cache configuration into the pool object #5983
Conversation
contificate
force-pushed
the
pool-cache-config
branch
2 times, most recently
from
6546b35 to
38a7397
Compare
|
Can we hide this fields from normal clients? It seems to me these should be internal configuration, and we don't want users meddling with these. |
|
I've updated this so that the related fields are |
In the sense of |
contificate
force-pushed
the
pool-cache-config
branch
from
38a7397 to
cd4a03e
Compare
contificate
force-pushed
the
pool-cache-config
branch
3 times, most recently
from
272d0b5 to
9bac528
Compare
|
I've added |
contificate
force-pushed
the
pool-cache-config
branch
2 times, most recently
from
530c750 to
6c4d34f
Compare
contificate
force-pushed
the
pool-cache-config
branch
2 times, most recently
from
a7ea1bd to
8d6bd54
Compare
| if capacity < 0L then | ||
| raise Api_errors.(Server_error (invalid_cache_size, [])) | ||
| else | ||
| Db.Pool.set_ext_auth_cache_size ~__context ~self ~value:capacity |
There was a problem hiding this comment.
Don't you want to call clear_external_auth_cache here (and in the following function) to ensure that changes get picked up immediately, without having to restart xapi?
There was a problem hiding this comment.
That could be the behaviour, if we wanted it. It's just quite a static setting overall, in that we don't support changing the size in a dynamic way that would imply the truncation of the cache's current contents at runtime. Kind of hoping these settings are fairly static and never played with.
You don't need to restart xapi though, as disable and re-enabling both external authentication generally and the cache feature has the effect of clearing the cache (upon disable). So the new changes would be picked up at the next external authentication attempt.
There was a problem hiding this comment.
OK, that's fine. If we were to actually publish this in the API docs, it would be good to mention this there (the need to disable+re-enable).
contificate
force-pushed
the
pool-cache-config
branch
from
8d6bd54 to
9e33301
Compare
Adds 3 new fields to the pool object: - ext_auth_cach_enabled: bool Specifies whether or not external authentication caching is enabled for this pool. The field is false by default. - ext_auth_cach_size: int Specifies the maximum capacity of the external authentication cache. Its default value is 50. - ext_auth_cache_expiry: int Specifies how long cached entries should remain cached. The default value is 300 seconds (5 minutes). Previously, these options were configured from xapi.conf, but in the case of designation of new pool coordinator (by autonomous means or otherwise), we would like for the feature's configuration to be adopted by new coordinator (without necessitating duplicate xapi configurations across hosts in the pool). The cache's contents itself will not be preserved, but caching will resume on the new coordinator if it was enabled on the previous coordinator. Signed-off-by: Colin James <colin.barr@cloud.com>
Signed-off-by: Colin James <colin.barr@cloud.com>
Replaces the usages of Xapi_globs within external authentication cache-related code to query the pool object for its configuration options. Signed-off-by: Colin James <colin.barr@cloud.com>
Modifies external authentication cache-related code to no longer rely on a globally-specified expiry time (TTL). Instead, caches are constructed with a given expiry time that they will endow entries with for the extent of their lifetime. There are now no usages of Xapi_globs in cache-related code, so those configuration options can be removed. Signed-off-by: Colin James <colin.barr@cloud.com>
Removes the global options that were once used to configure the external authentication cache. This is safe to do as the cache now configures itself from the pool object. Adds a few lines of debug messages. Signed-off-by: Colin James <colin.barr@cloud.com>
contificate
force-pushed
the
pool-cache-config
branch
from
9e33301 to
1c00a73
Compare
These simple changes move configuration options into the
poolobject.I suspect there are better approaches (naming and convention wise) than what is done here, and I'm happy to rebase those changes in. This feature has been tested manually and works. XenRT tests that exercise this code path properly are still in progress.
In future, I wonder if there are less invasive ways we could consider for providing feature configuration (rather than adding more fields to extant objects, whose database records are already overly large from the content they store). Perhaps we could introduce a variant datatype to the datamodel (akin to DBus'
Variant) such that fields similar toother_config : (string -> string) mapcould be more adequately typed (config : (string -> variant) map). This adds a kind of dynamic - transient - nature to the configuration that we may not want, though.