Re: [SSSD] [PATCHES] sss_userdel and sss_groupdel should invalidate mmap cache
On 11/26/2012 05:43 PM, Simo Sorce wrote:
On Thu, 2012-11-22 at 09:52 +0100, Michal Židek wrote:
On 11/21/2012 04:41 PM, Simo Sorce wrote:
On Wed, 2012-11-21 at 15:56 +0100, Michal Židek wrote:
On 11/21/2012 02:31 PM, Simo Sorce wrote:
On Tue, 2012-11-20 at 14:29 +0100, Michal Židek wrote:
On 11/20/2012 02:22 PM, Michal Židek wrote:
Patch 1: sss_cache refactor. See patch description for more details.
Patch 2: Remove mmap cache properly in sss_userdel and sss_groupdel
It looks a bit excessive to remove the whole cache when you change 1
user, would be probably better to fix this by a slightly more complex
construct.
After you delete the data from the ldb cache you could query sssd_nss
for the user name. And make sure that sssd_nss also searches the mc
cache and remove the entry if it is found there on user lookup failure
(we can later on use this feature to implement negative caching at the
mc layer).
Simo.
Hmm... we would still need to handle the situation when sssd_nss is not
running. I guess removing the entire mmap cache here is just fine.
What's the point of adding or deleting users if sssd is not running ?
If you want to simply wipe the cache we have a specific tool for that.
But, to the case when sssd_nss is running. Does sss_nss_make_request()
only ldb requests? Or first mc then ldb?
Only ldb, you need to add a mc request as an additional check, if the
ldb search tunrs out empty.
BAsically in the place where we do the negative caching now you'd add a
call to a helper function to check and invalidate the mc cache for the
specific user as well.
I am looking at the client code
now and it looks like this function is called when mc lookup did not
succeeded... so my guess is, that it only requests for ldb data. If so,
then this would be the function to use here.
It would be like:
... in the tool's code ...
sss_nss_make_request(SSS_NSS_GETPWNAM, ...);
... somewhere inside sssd_nss code ...
if (no_results_in_ldb) {
invalidate rec in mc
}
Exactly.
Since the invalidation takes place inside sssd_nss process, no new
synchronization between processes should be needed (the barriers are
already there). But this would only work if sss_nss_make_request
communicates with ldb only (which I think it does).
Was this your suggestion?
Yes, precisely.
Simo.
However, it might still be good to push the originally proposed patches
as a temporary solution (or at least the first of them because the
sss_cache code is more readable after the refactor).
Yes please rebase and resubmit the first patch on its own, it is already
acked.
For the second one, it seem we need a general mechanism to clear
individual expired entries from the mc, I am working with Jakub on that,
so once it is ready we should be able to wire it up from sss_* utils as
well.
So defer second for now.
Simo.
Sending the first patch only. Rebased against current master and updated
commit message.
Thanks
Michal
From 802b4a78611fd512fe7050a5cb339aa8c7d1fc09 Mon Sep 17 00:00:00 2001
From: Michal Zidek mzi...@redhat.com
Date: Tue, 20 Nov 2012 13:52:12 +0100
Subject: [PATCH] sss_cache: Small refactor.
The logic that checks if sssd_nss is running and then
sends SIGHUP to monitor or removes the caches was moved
to a function sss_memcache_clear_all() and made public in
tools_util.h.
---
src/tools/sss_cache.c | 61 +++--
src/tools/tools_util.c | 67 ++
src/tools/tools_util.h | 2 ++
3 files changed, 72 insertions(+), 58 deletions(-)
diff --git a/src/tools/sss_cache.c b/src/tools/sss_cache.c
index 368b1df..9305db5 100644
--- a/src/tools/sss_cache.c
+++ b/src/tools/sss_cache.c
@@ -92,7 +92,6 @@ errno_t invalidate_entry(TALLOC_CTX *ctx, struct sysdb_ctx *sysdb,
bool invalidate_entries(TALLOC_CTX *ctx, struct sysdb_ctx *sysdb,
enum sss_cache_entry entry_type, const char *filter,
const char *name);
-static int clear_fastcache(bool *sssd_nss_is_off);
int main(int argc, const char *argv[])
{
@@ -100,9 +99,7 @@ int main(int argc, const char *argv[])
struct cache_tool_ctx *tctx = NULL;
struct sysdb_ctx *sysdb;
int i;
-bool sssd_nss_is_off;
bool skipped = true;
-FILE *clear_mc_flag;
ret = init_context(argc, argv, tctx);
if (ret != EOK) {
@@ -145,71 +142,19 @@ int main(int argc, const char *argv[])
ret = ENOENT;
goto done;
} else {
-ret = clear_fastcache(sssd_nss_is_off);
+ret = sss_memcache_clear_all();
if (ret != EOK) {
-DEBUG(SSSDBG_CRIT_FAILURE, (Failed to clear caches.\n));
+DEBUG(SSSDBG_CRIT_FAILURE, (Failed to clear memory cache.\n));
goto done;
}
-if (!sssd_nss_is_off) {
-/* sssd_nss is running - signal monitor to invalidate fastcache */
-clear_mc_flag = fopen(SSS_NSS_MCACHE_DIR/CLEAR_MC_FLAG, w);
-if
Re: [SSSD] [PATCHES] sss_userdel and sss_groupdel should invalidate mmap cache
On Tue, 2012-11-27 at 11:02 +0100, Michal Židek wrote:
On 11/26/2012 05:43 PM, Simo Sorce wrote:
On Thu, 2012-11-22 at 09:52 +0100, Michal Židek wrote:
On 11/21/2012 04:41 PM, Simo Sorce wrote:
On Wed, 2012-11-21 at 15:56 +0100, Michal Židek wrote:
On 11/21/2012 02:31 PM, Simo Sorce wrote:
On Tue, 2012-11-20 at 14:29 +0100, Michal Židek wrote:
On 11/20/2012 02:22 PM, Michal Židek wrote:
Patch 1: sss_cache refactor. See patch description for more details.
Patch 2: Remove mmap cache properly in sss_userdel and sss_groupdel
It looks a bit excessive to remove the whole cache when you change 1
user, would be probably better to fix this by a slightly more complex
construct.
After you delete the data from the ldb cache you could query sssd_nss
for the user name. And make sure that sssd_nss also searches the mc
cache and remove the entry if it is found there on user lookup failure
(we can later on use this feature to implement negative caching at the
mc layer).
Simo.
Hmm... we would still need to handle the situation when sssd_nss is not
running. I guess removing the entire mmap cache here is just fine.
What's the point of adding or deleting users if sssd is not running ?
If you want to simply wipe the cache we have a specific tool for that.
But, to the case when sssd_nss is running. Does sss_nss_make_request()
only ldb requests? Or first mc then ldb?
Only ldb, you need to add a mc request as an additional check, if the
ldb search tunrs out empty.
BAsically in the place where we do the negative caching now you'd add a
call to a helper function to check and invalidate the mc cache for the
specific user as well.
I am looking at the client code
now and it looks like this function is called when mc lookup did not
succeeded... so my guess is, that it only requests for ldb data. If so,
then this would be the function to use here.
It would be like:
... in the tool's code ...
sss_nss_make_request(SSS_NSS_GETPWNAM, ...);
... somewhere inside sssd_nss code ...
if (no_results_in_ldb) {
invalidate rec in mc
}
Exactly.
Since the invalidation takes place inside sssd_nss process, no new
synchronization between processes should be needed (the barriers are
already there). But this would only work if sss_nss_make_request
communicates with ldb only (which I think it does).
Was this your suggestion?
Yes, precisely.
Simo.
However, it might still be good to push the originally proposed patches
as a temporary solution (or at least the first of them because the
sss_cache code is more readable after the refactor).
Yes please rebase and resubmit the first patch on its own, it is already
acked.
For the second one, it seem we need a general mechanism to clear
individual expired entries from the mc, I am working with Jakub on that,
so once it is ready we should be able to wire it up from sss_* utils as
well.
So defer second for now.
Simo.
Sending the first patch only. Rebased against current master and updated
commit message.
Ack
Simo.
--
Simo Sorce * Red Hat, Inc * New York
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
Re: [SSSD] [PATCHES] sss_userdel and sss_groupdel should invalidate mmap cache
On Thu, 2012-11-22 at 09:52 +0100, Michal Židek wrote:
On 11/21/2012 04:41 PM, Simo Sorce wrote:
On Wed, 2012-11-21 at 15:56 +0100, Michal Židek wrote:
On 11/21/2012 02:31 PM, Simo Sorce wrote:
On Tue, 2012-11-20 at 14:29 +0100, Michal Židek wrote:
On 11/20/2012 02:22 PM, Michal Židek wrote:
Patch 1: sss_cache refactor. See patch description for more details.
Patch 2: Remove mmap cache properly in sss_userdel and sss_groupdel
It looks a bit excessive to remove the whole cache when you change 1
user, would be probably better to fix this by a slightly more complex
construct.
After you delete the data from the ldb cache you could query sssd_nss
for the user name. And make sure that sssd_nss also searches the mc
cache and remove the entry if it is found there on user lookup failure
(we can later on use this feature to implement negative caching at the
mc layer).
Simo.
Hmm... we would still need to handle the situation when sssd_nss is not
running. I guess removing the entire mmap cache here is just fine.
What's the point of adding or deleting users if sssd is not running ?
If you want to simply wipe the cache we have a specific tool for that.
But, to the case when sssd_nss is running. Does sss_nss_make_request()
only ldb requests? Or first mc then ldb?
Only ldb, you need to add a mc request as an additional check, if the
ldb search tunrs out empty.
BAsically in the place where we do the negative caching now you'd add a
call to a helper function to check and invalidate the mc cache for the
specific user as well.
I am looking at the client code
now and it looks like this function is called when mc lookup did not
succeeded... so my guess is, that it only requests for ldb data. If so,
then this would be the function to use here.
It would be like:
... in the tool's code ...
sss_nss_make_request(SSS_NSS_GETPWNAM, ...);
... somewhere inside sssd_nss code ...
if (no_results_in_ldb) {
invalidate rec in mc
}
Exactly.
Since the invalidation takes place inside sssd_nss process, no new
synchronization between processes should be needed (the barriers are
already there). But this would only work if sss_nss_make_request
communicates with ldb only (which I think it does).
Was this your suggestion?
Yes, precisely.
Simo.
However, it might still be good to push the originally proposed patches
as a temporary solution (or at least the first of them because the
sss_cache code is more readable after the refactor).
Yes please rebase and resubmit the first patch on its own, it is already
acked.
For the second one, it seem we need a general mechanism to clear
individual expired entries from the mc, I am working with Jakub on that,
so once it is ready we should be able to wire it up from sss_* utils as
well.
So defer second for now.
Simo.
--
Simo Sorce * Red Hat, Inc * New York
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
Re: [SSSD] [PATCHES] sss_userdel and sss_groupdel should invalidate mmap cache
On 11/21/2012 04:41 PM, Simo Sorce wrote:
On Wed, 2012-11-21 at 15:56 +0100, Michal Židek wrote:
On 11/21/2012 02:31 PM, Simo Sorce wrote:
On Tue, 2012-11-20 at 14:29 +0100, Michal Židek wrote:
On 11/20/2012 02:22 PM, Michal Židek wrote:
Patch 1: sss_cache refactor. See patch description for more details.
Patch 2: Remove mmap cache properly in sss_userdel and sss_groupdel
It looks a bit excessive to remove the whole cache when you change 1
user, would be probably better to fix this by a slightly more complex
construct.
After you delete the data from the ldb cache you could query sssd_nss
for the user name. And make sure that sssd_nss also searches the mc
cache and remove the entry if it is found there on user lookup failure
(we can later on use this feature to implement negative caching at the
mc layer).
Simo.
Hmm... we would still need to handle the situation when sssd_nss is not
running. I guess removing the entire mmap cache here is just fine.
What's the point of adding or deleting users if sssd is not running ?
If you want to simply wipe the cache we have a specific tool for that.
But, to the case when sssd_nss is running. Does sss_nss_make_request()
only ldb requests? Or first mc then ldb?
Only ldb, you need to add a mc request as an additional check, if the
ldb search tunrs out empty.
BAsically in the place where we do the negative caching now you'd add a
call to a helper function to check and invalidate the mc cache for the
specific user as well.
I am looking at the client code
now and it looks like this function is called when mc lookup did not
succeeded... so my guess is, that it only requests for ldb data. If so,
then this would be the function to use here.
It would be like:
... in the tool's code ...
sss_nss_make_request(SSS_NSS_GETPWNAM, ...);
... somewhere inside sssd_nss code ...
if (no_results_in_ldb) {
invalidate rec in mc
}
Exactly.
Since the invalidation takes place inside sssd_nss process, no new
synchronization between processes should be needed (the barriers are
already there). But this would only work if sss_nss_make_request
communicates with ldb only (which I think it does).
Was this your suggestion?
Yes, precisely.
Simo.
However, it might still be good to push the originally proposed patches
as a temporary solution (or at least the first of them because the
sss_cache code is more readable after the refactor).
Thanks
Michal
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
Re: [SSSD] [PATCHES] sss_userdel and sss_groupdel should invalidate mmap cache
On 11/20/2012 02:29 PM, Michal Židek wrote: On 11/20/2012 02:22 PM, Michal Židek wrote: Patch 1: sss_cache refactor. See patch description for more details. Patch 2: Remove mmap cache properly in sss_userdel and sss_groupdel Patches are attached. Thanks Michal ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel https://fedorahosted.org/sssd/ticket/1659 I modified the commit message of second patch to include link to the ticket. Thanks Michal ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel [PATCH 1/2]: nitpick: git am warns about empty line at EOF +ret = sss_memcache_clear_all(); +DEBUG(SSSDBG_CRIT_FAILURE, (Failed to clear memory cache.\n)); +goto done; the debug message will print every time, aren't you supposed to test ret and print it on != EOK ? [PATCH 2/2]: Ack -- Ondrej Kos Associate Software Engineer Identity Management Red Hat Czech phone: +420-532-294-558 cell: +420-736-417-909 ext: 82-62558 loc: 1013 Brno 1 office irc: okos @ #brno ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
Re: [SSSD] [PATCHES] sss_userdel and sss_groupdel should invalidate mmap cache
On 11/21/2012 01:06 PM, Ondrej Kos wrote:
On 11/20/2012 02:29 PM, Michal Židek wrote:
On 11/20/2012 02:22 PM, Michal Židek wrote:
Patch 1: sss_cache refactor. See patch description for more details.
Patch 2: Remove mmap cache properly in sss_userdel and sss_groupdel
Patches are attached.
Thanks
Michal
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
https://fedorahosted.org/sssd/ticket/1659
I modified the commit message of second patch to include link to the
ticket.
Thanks
Michal
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
[PATCH 1/2]:
nitpick: git am warns about empty line at EOF
+ret = sss_memcache_clear_all();
+DEBUG(SSSDBG_CRIT_FAILURE, (Failed to clear memory cache.\n));
+goto done;
the debug message will print every time, aren't you supposed to test ret
and print it on != EOK ?
Oops. Sure, this was wrong.
[PATCH 2/2]:
Ack
New patches attached.
Michal
From bc475b9a15104fd4eb20a10007a64f914bb11d69 Mon Sep 17 00:00:00 2001
From: Michal Zidek mzi...@redhat.com
Date: Tue, 20 Nov 2012 13:52:12 +0100
Subject: [PATCH 1/2] sss_cache: Small refactor.
The logic that checks if sssd_nss is running and then
sends SIGHUP to monitor or removes the caches was moved
to a function sss_memcache_clear_all() and made public in
tools_util.h. This function can be used by other tools
that need memory cache to be invalidated like sss_userdel
and sss_groupdel.
---
src/tools/sss_cache.c | 61 +++--
src/tools/tools_util.c | 67 ++
src/tools/tools_util.h | 2 ++
3 files changed, 72 insertions(+), 58 deletions(-)
diff --git a/src/tools/sss_cache.c b/src/tools/sss_cache.c
index 368b1df..9305db5 100644
--- a/src/tools/sss_cache.c
+++ b/src/tools/sss_cache.c
@@ -92,7 +92,6 @@ errno_t invalidate_entry(TALLOC_CTX *ctx, struct sysdb_ctx *sysdb,
bool invalidate_entries(TALLOC_CTX *ctx, struct sysdb_ctx *sysdb,
enum sss_cache_entry entry_type, const char *filter,
const char *name);
-static int clear_fastcache(bool *sssd_nss_is_off);
int main(int argc, const char *argv[])
{
@@ -100,9 +99,7 @@ int main(int argc, const char *argv[])
struct cache_tool_ctx *tctx = NULL;
struct sysdb_ctx *sysdb;
int i;
-bool sssd_nss_is_off;
bool skipped = true;
-FILE *clear_mc_flag;
ret = init_context(argc, argv, tctx);
if (ret != EOK) {
@@ -145,71 +142,19 @@ int main(int argc, const char *argv[])
ret = ENOENT;
goto done;
} else {
-ret = clear_fastcache(sssd_nss_is_off);
+ret = sss_memcache_clear_all();
if (ret != EOK) {
-DEBUG(SSSDBG_CRIT_FAILURE, (Failed to clear caches.\n));
+DEBUG(SSSDBG_CRIT_FAILURE, (Failed to clear memory cache.\n));
goto done;
}
-if (!sssd_nss_is_off) {
-/* sssd_nss is running - signal monitor to invalidate fastcache */
-clear_mc_flag = fopen(SSS_NSS_MCACHE_DIR/CLEAR_MC_FLAG, w);
-if (clear_mc_flag == NULL) {
-DEBUG(SSSDBG_CRIT_FAILURE,
- (Failed to create clear_mc_flag file.
- Memory cache will not be cleared.\n));
-goto done;
-}
-ret = fclose(clear_mc_flag);
-if (ret != 0) {
-ret = errno;
-DEBUG(SSSDBG_CRIT_FAILURE,
- (Unable to close file descriptor: %s\n,
- strerror(ret)));
-goto done;
-}
-
-DEBUG(SSSDBG_TRACE_FUNC, (Sending SIGHUP to monitor.\n));
-ret = signal_sssd(SIGHUP);
-if (ret != EOK) {
-DEBUG(SSSDBG_CRIT_FAILURE,
- (Failed to send SIGHUP to monitor.\n));
-goto done;
-}
-}
}
+ret = EOK;
done:
if (tctx) talloc_free(tctx);
return ret;
}
-static int clear_fastcache(bool *sssd_nss_is_off)
-{
-int ret;
-ret = sss_memcache_invalidate(SSS_NSS_MCACHE_DIR/passwd);
-if (ret != EOK) {
-if (ret == EACCES) {
-*sssd_nss_is_off = false;
-return EOK;
-} else {
-return ret;
-}
-}
-
-ret = sss_memcache_invalidate(SSS_NSS_MCACHE_DIR/group);
-if (ret != EOK) {
-if (ret == EACCES) {
-*sssd_nss_is_off = false;
-return EOK;
-} else {
-return ret;
-}
-}
-
-*sssd_nss_is_off = true;
-return EOK;
-}
-
bool invalidate_entries(TALLOC_CTX *ctx, struct sysdb_ctx *sysdb,
enum sss_cache_entry entry_type, const char *filter,
Re: [SSSD] [PATCHES] sss_userdel and sss_groupdel should invalidate mmap cache
On 11/21/2012 01:25 PM, Michal Židek wrote: On 11/21/2012 01:06 PM, Ondrej Kos wrote: On 11/20/2012 02:29 PM, Michal Židek wrote: On 11/20/2012 02:22 PM, Michal Židek wrote: Patch 1: sss_cache refactor. See patch description for more details. Patch 2: Remove mmap cache properly in sss_userdel and sss_groupdel Patches are attached. Thanks Michal ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel https://fedorahosted.org/sssd/ticket/1659 I modified the commit message of second patch to include link to the ticket. Thanks Michal ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel [PATCH 1/2]: nitpick: git am warns about empty line at EOF +ret = sss_memcache_clear_all(); +DEBUG(SSSDBG_CRIT_FAILURE, (Failed to clear memory cache.\n)); +goto done; the debug message will print every time, aren't you supposed to test ret and print it on != EOK ? Oops. Sure, this was wrong. [PATCH 2/2]: Ack New patches attached. Michal ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel Thanks, Ack on both then. Ondra -- Ondrej Kos Associate Software Engineer Identity Management Red Hat Czech phone: +420-532-294-558 cell: +420-736-417-909 ext: 82-62558 loc: 1013 Brno 1 office irc: okos @ #brno ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
Re: [SSSD] [PATCHES] sss_userdel and sss_groupdel should invalidate mmap cache
On Tue, 2012-11-20 at 14:29 +0100, Michal Židek wrote: On 11/20/2012 02:22 PM, Michal Židek wrote: Patch 1: sss_cache refactor. See patch description for more details. Patch 2: Remove mmap cache properly in sss_userdel and sss_groupdel It looks a bit excessive to remove the whole cache when you change 1 user, would be probably better to fix this by a slightly more complex construct. After you delete the data from the ldb cache you could query sssd_nss for the user name. And make sure that sssd_nss also searches the mc cache and remove the entry if it is found there on user lookup failure (we can later on use this feature to implement negative caching at the mc layer). Simo. -- Simo Sorce * Red Hat, Inc * New York ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
Re: [SSSD] [PATCHES] sss_userdel and sss_groupdel should invalidate mmap cache
On 11/21/2012 02:31 PM, Simo Sorce wrote:
On Tue, 2012-11-20 at 14:29 +0100, Michal Židek wrote:
On 11/20/2012 02:22 PM, Michal Židek wrote:
Patch 1: sss_cache refactor. See patch description for more details.
Patch 2: Remove mmap cache properly in sss_userdel and sss_groupdel
It looks a bit excessive to remove the whole cache when you change 1
user, would be probably better to fix this by a slightly more complex
construct.
After you delete the data from the ldb cache you could query sssd_nss
for the user name. And make sure that sssd_nss also searches the mc
cache and remove the entry if it is found there on user lookup failure
(we can later on use this feature to implement negative caching at the
mc layer).
Simo.
Hmm... we would still need to handle the situation when sssd_nss is not
running. I guess removing the entire mmap cache here is just fine.
But, to the case when sssd_nss is running. Does sss_nss_make_request()
only ldb requests? Or first mc then ldb? I am looking at the client code
now and it looks like this function is called when mc lookup did not
succeeded... so my guess is, that it only requests for ldb data. If so,
then this would be the function to use here.
It would be like:
... in the tool's code ...
sss_nss_make_request(SSS_NSS_GETPWNAM, ...);
... somewhere inside sssd_nss code ...
if (no_results_in_ldb) {
invalidate rec in mc
}
Since the invalidation takes place inside sssd_nss process, no new
synchronization between processes should be needed (the barriers are
already there). But this would only work if sss_nss_make_request
communicates with ldb only (which I think it does).
Was this your suggestion?
Thanks
Michal
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
Re: [SSSD] [PATCHES] sss_userdel and sss_groupdel should invalidate mmap cache
On Wed, 2012-11-21 at 15:56 +0100, Michal Židek wrote:
On 11/21/2012 02:31 PM, Simo Sorce wrote:
On Tue, 2012-11-20 at 14:29 +0100, Michal Židek wrote:
On 11/20/2012 02:22 PM, Michal Židek wrote:
Patch 1: sss_cache refactor. See patch description for more details.
Patch 2: Remove mmap cache properly in sss_userdel and sss_groupdel
It looks a bit excessive to remove the whole cache when you change 1
user, would be probably better to fix this by a slightly more complex
construct.
After you delete the data from the ldb cache you could query sssd_nss
for the user name. And make sure that sssd_nss also searches the mc
cache and remove the entry if it is found there on user lookup failure
(we can later on use this feature to implement negative caching at the
mc layer).
Simo.
Hmm... we would still need to handle the situation when sssd_nss is not
running. I guess removing the entire mmap cache here is just fine.
What's the point of adding or deleting users if sssd is not running ?
If you want to simply wipe the cache we have a specific tool for that.
But, to the case when sssd_nss is running. Does sss_nss_make_request()
only ldb requests? Or first mc then ldb?
Only ldb, you need to add a mc request as an additional check, if the
ldb search tunrs out empty.
BAsically in the place where we do the negative caching now you'd add a
call to a helper function to check and invalidate the mc cache for the
specific user as well.
I am looking at the client code
now and it looks like this function is called when mc lookup did not
succeeded... so my guess is, that it only requests for ldb data. If so,
then this would be the function to use here.
It would be like:
... in the tool's code ...
sss_nss_make_request(SSS_NSS_GETPWNAM, ...);
... somewhere inside sssd_nss code ...
if (no_results_in_ldb) {
invalidate rec in mc
}
Exactly.
Since the invalidation takes place inside sssd_nss process, no new
synchronization between processes should be needed (the barriers are
already there). But this would only work if sss_nss_make_request
communicates with ldb only (which I think it does).
Was this your suggestion?
Yes, precisely.
Simo.
--
Simo Sorce * Red Hat, Inc * New York
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
[SSSD] [PATCHES] sss_userdel and sss_groupdel should invalidate mmap cache
Patch 1: sss_cache refactor. See patch description for more details.
Patch 2: Remove mmap cache properly in sss_userdel and sss_groupdel
Patches are attached.
Thanks
Michal
From c02ae437e619ab97fb50123706ec1dbe2d920e72 Mon Sep 17 00:00:00 2001
From: Michal Zidek mzi...@redhat.com
Date: Tue, 20 Nov 2012 13:52:12 +0100
Subject: [PATCH 1/2] sss_cache: Small refactor.
The logic that checks if sssd_nss is running and then
sends SIGHUP to monitor or removes the caches was moved
to a function sss_memcache_clear_all() and made public in
tools_util.h. This function can be used by other tools
that need memory cache to be invalidated like sss_userdel
and sss_groupdel.
---
src/tools/sss_cache.c | 65 +++
src/tools/tools_util.c | 69 ++
src/tools/tools_util.h | 2 ++
3 files changed, 75 insertions(+), 61 deletions(-)
diff --git a/src/tools/sss_cache.c b/src/tools/sss_cache.c
index 368b1df..703c033 100644
--- a/src/tools/sss_cache.c
+++ b/src/tools/sss_cache.c
@@ -92,7 +92,6 @@ errno_t invalidate_entry(TALLOC_CTX *ctx, struct sysdb_ctx *sysdb,
bool invalidate_entries(TALLOC_CTX *ctx, struct sysdb_ctx *sysdb,
enum sss_cache_entry entry_type, const char *filter,
const char *name);
-static int clear_fastcache(bool *sssd_nss_is_off);
int main(int argc, const char *argv[])
{
@@ -100,9 +99,7 @@ int main(int argc, const char *argv[])
struct cache_tool_ctx *tctx = NULL;
struct sysdb_ctx *sysdb;
int i;
-bool sssd_nss_is_off;
bool skipped = true;
-FILE *clear_mc_flag;
ret = init_context(argc, argv, tctx);
if (ret != EOK) {
@@ -145,71 +142,17 @@ int main(int argc, const char *argv[])
ret = ENOENT;
goto done;
} else {
-ret = clear_fastcache(sssd_nss_is_off);
-if (ret != EOK) {
-DEBUG(SSSDBG_CRIT_FAILURE, (Failed to clear caches.\n));
-goto done;
-}
-if (!sssd_nss_is_off) {
-/* sssd_nss is running - signal monitor to invalidate fastcache */
-clear_mc_flag = fopen(SSS_NSS_MCACHE_DIR/CLEAR_MC_FLAG, w);
-if (clear_mc_flag == NULL) {
-DEBUG(SSSDBG_CRIT_FAILURE,
- (Failed to create clear_mc_flag file.
- Memory cache will not be cleared.\n));
-goto done;
-}
-ret = fclose(clear_mc_flag);
-if (ret != 0) {
-ret = errno;
-DEBUG(SSSDBG_CRIT_FAILURE,
- (Unable to close file descriptor: %s\n,
- strerror(ret)));
-goto done;
-}
-
-DEBUG(SSSDBG_TRACE_FUNC, (Sending SIGHUP to monitor.\n));
-ret = signal_sssd(SIGHUP);
-if (ret != EOK) {
-DEBUG(SSSDBG_CRIT_FAILURE,
- (Failed to send SIGHUP to monitor.\n));
-goto done;
-}
-}
+ret = sss_memcache_clear_all();
+DEBUG(SSSDBG_CRIT_FAILURE, (Failed to clear memory cache.\n));
+goto done;
}
+ret = EOK;
done:
if (tctx) talloc_free(tctx);
return ret;
}
-static int clear_fastcache(bool *sssd_nss_is_off)
-{
-int ret;
-ret = sss_memcache_invalidate(SSS_NSS_MCACHE_DIR/passwd);
-if (ret != EOK) {
-if (ret == EACCES) {
-*sssd_nss_is_off = false;
-return EOK;
-} else {
-return ret;
-}
-}
-
-ret = sss_memcache_invalidate(SSS_NSS_MCACHE_DIR/group);
-if (ret != EOK) {
-if (ret == EACCES) {
-*sssd_nss_is_off = false;
-return EOK;
-} else {
-return ret;
-}
-}
-
-*sssd_nss_is_off = true;
-return EOK;
-}
-
bool invalidate_entries(TALLOC_CTX *ctx, struct sysdb_ctx *sysdb,
enum sss_cache_entry entry_type, const char *filter,
const char *name)
diff --git a/src/tools/tools_util.c b/src/tools/tools_util.c
index 73e9413..7054359 100644
--- a/src/tools/tools_util.c
+++ b/src/tools/tools_util.c
@@ -766,3 +766,72 @@ done:
}
return ret;
}
+
+static int clear_fastcache(bool *sssd_nss_is_off)
+{
+int ret;
+ret = sss_memcache_invalidate(SSS_NSS_MCACHE_DIR/passwd);
+if (ret != EOK) {
+if (ret == EACCES) {
+*sssd_nss_is_off = false;
+return EOK;
+} else {
+return ret;
+}
+}
+
+ret = sss_memcache_invalidate(SSS_NSS_MCACHE_DIR/group);
+if (ret != EOK) {
+if (ret == EACCES) {
+*sssd_nss_is_off = false;
+return EOK;
+} else {
+return ret;
+}
+}
+
+*sssd_nss_is_off = true;
+return EOK;
+}
+
+errno_t sss_memcache_clear_all(void)
+{
+errno_t ret;
+bool sssd_nss_is_off = false;
+FILE
Re: [SSSD] [PATCHES] sss_userdel and sss_groupdel should invalidate mmap cache
On 11/20/2012 02:22 PM, Michal Židek wrote:
Patch 1: sss_cache refactor. See patch description for more details.
Patch 2: Remove mmap cache properly in sss_userdel and sss_groupdel
Patches are attached.
Thanks
Michal
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
https://fedorahosted.org/sssd/ticket/1659
I modified the commit message of second patch to include link to the ticket.
Thanks
Michal
From c02ae437e619ab97fb50123706ec1dbe2d920e72 Mon Sep 17 00:00:00 2001
From: Michal Zidek mzi...@redhat.com
Date: Tue, 20 Nov 2012 13:52:12 +0100
Subject: [PATCH 1/2] sss_cache: Small refactor.
The logic that checks if sssd_nss is running and then
sends SIGHUP to monitor or removes the caches was moved
to a function sss_memcache_clear_all() and made public in
tools_util.h. This function can be used by other tools
that need memory cache to be invalidated like sss_userdel
and sss_groupdel.
---
src/tools/sss_cache.c | 65 +++
src/tools/tools_util.c | 69 ++
src/tools/tools_util.h | 2 ++
3 files changed, 75 insertions(+), 61 deletions(-)
diff --git a/src/tools/sss_cache.c b/src/tools/sss_cache.c
index 368b1df..703c033 100644
--- a/src/tools/sss_cache.c
+++ b/src/tools/sss_cache.c
@@ -92,7 +92,6 @@ errno_t invalidate_entry(TALLOC_CTX *ctx, struct sysdb_ctx *sysdb,
bool invalidate_entries(TALLOC_CTX *ctx, struct sysdb_ctx *sysdb,
enum sss_cache_entry entry_type, const char *filter,
const char *name);
-static int clear_fastcache(bool *sssd_nss_is_off);
int main(int argc, const char *argv[])
{
@@ -100,9 +99,7 @@ int main(int argc, const char *argv[])
struct cache_tool_ctx *tctx = NULL;
struct sysdb_ctx *sysdb;
int i;
-bool sssd_nss_is_off;
bool skipped = true;
-FILE *clear_mc_flag;
ret = init_context(argc, argv, tctx);
if (ret != EOK) {
@@ -145,71 +142,17 @@ int main(int argc, const char *argv[])
ret = ENOENT;
goto done;
} else {
-ret = clear_fastcache(sssd_nss_is_off);
-if (ret != EOK) {
-DEBUG(SSSDBG_CRIT_FAILURE, (Failed to clear caches.\n));
-goto done;
-}
-if (!sssd_nss_is_off) {
-/* sssd_nss is running - signal monitor to invalidate fastcache */
-clear_mc_flag = fopen(SSS_NSS_MCACHE_DIR/CLEAR_MC_FLAG, w);
-if (clear_mc_flag == NULL) {
-DEBUG(SSSDBG_CRIT_FAILURE,
- (Failed to create clear_mc_flag file.
- Memory cache will not be cleared.\n));
-goto done;
-}
-ret = fclose(clear_mc_flag);
-if (ret != 0) {
-ret = errno;
-DEBUG(SSSDBG_CRIT_FAILURE,
- (Unable to close file descriptor: %s\n,
- strerror(ret)));
-goto done;
-}
-
-DEBUG(SSSDBG_TRACE_FUNC, (Sending SIGHUP to monitor.\n));
-ret = signal_sssd(SIGHUP);
-if (ret != EOK) {
-DEBUG(SSSDBG_CRIT_FAILURE,
- (Failed to send SIGHUP to monitor.\n));
-goto done;
-}
-}
+ret = sss_memcache_clear_all();
+DEBUG(SSSDBG_CRIT_FAILURE, (Failed to clear memory cache.\n));
+goto done;
}
+ret = EOK;
done:
if (tctx) talloc_free(tctx);
return ret;
}
-static int clear_fastcache(bool *sssd_nss_is_off)
-{
-int ret;
-ret = sss_memcache_invalidate(SSS_NSS_MCACHE_DIR/passwd);
-if (ret != EOK) {
-if (ret == EACCES) {
-*sssd_nss_is_off = false;
-return EOK;
-} else {
-return ret;
-}
-}
-
-ret = sss_memcache_invalidate(SSS_NSS_MCACHE_DIR/group);
-if (ret != EOK) {
-if (ret == EACCES) {
-*sssd_nss_is_off = false;
-return EOK;
-} else {
-return ret;
-}
-}
-
-*sssd_nss_is_off = true;
-return EOK;
-}
-
bool invalidate_entries(TALLOC_CTX *ctx, struct sysdb_ctx *sysdb,
enum sss_cache_entry entry_type, const char *filter,
const char *name)
diff --git a/src/tools/tools_util.c b/src/tools/tools_util.c
index 73e9413..7054359 100644
--- a/src/tools/tools_util.c
+++ b/src/tools/tools_util.c
@@ -766,3 +766,72 @@ done:
}
return ret;
}
+
+static int clear_fastcache(bool *sssd_nss_is_off)
+{
+int ret;
+ret = sss_memcache_invalidate(SSS_NSS_MCACHE_DIR/passwd);
+if (ret != EOK) {
+if (ret == EACCES) {
+*sssd_nss_is_off = false;
+return EOK;
+} else {
+return ret;
+}
+}
+
+ret =
