[PATCH] Add online callback only once for TGT renewal

[Sumit Bose]

Hi,

I confused the online callbacks with tevent timers when adding them to
the TGT renewal task with the effect that a new renewal timer was added
every time the system switched back from offline to online. Thanks to
Kaushik for catching this. The attached patch should fix it.

bye,
Sumit

[Dmitri Pal]

On 05/27/2011 07:02 AM, Sumit Bose wrote:
> Hi,
>
> I confused the online callbacks with tevent timers when adding them to
> the TGT renewal task with the effect that a new renewal timer was added
> every time the system switched back from offline to online. Thanks to
> Kaushik for catching this. The attached patch should fix it.
>
> bye,
> Sumit
>
> sssd-devel mailing list
> [e-mail]
> https://fedorahosted.org/mailman/listinfo/sssd-devel

What is the impact of this?
Is it a problem with the shipped version?
Can it cause a leak or other issues?
Should we add it to errata?

[Stephen Gallagher]

On Fri, 2011-05-27 at 09:58 -0400, Dmitri Pal wrote:
> On 05/27/2011 07:02 AM, Sumit Bose wrote:
> > Hi,
> >
> > I confused the online callbacks with tevent timers when adding them to
> > the TGT renewal task with the effect that a new renewal timer was added
> > every time the system switched back from offline to online. Thanks to
> > Kaushik for catching this. The attached patch should fix it.
> What is the impact of this?

The renewals were occurring multiple times. This meant increased load on
the Kerberos server as well as extra log messages in SSSD's DEBUG logs
(as noticed by Kaushik)
> Is it a problem with the shipped version?

Yes
> Can it cause a leak or other issues?

It is technically a leak, but it is a very small amount of memory (one
additional tevent_timer object for each time we switched from offline
mode to online mode).
> Should we add it to errata?

I think that's should be decided by QE. I'm on the fence. The memory
leak doesn't concern me (since it's small) but the additional load on
the KDC might be worth the errata.

[Sumit Bose]

On Fri, May 27, 2011 at 10:27:48AM -0400, Stephen Gallagher wrote:
> On Fri, 2011-05-27 at 09:58 -0400, Dmitri Pal wrote:
> > On 05/27/2011 07:02 AM, Sumit Bose wrote:
> > > Hi,
> > >
> > > I confused the online callbacks with tevent timers when adding them to
> > > the TGT renewal task with the effect that a new renewal timer was added
> > > every time the system switched back from offline to online. Thanks to
> > > Kaushik for catching this. The attached patch should fix it.
> > What is the impact of this?
>
> The renewals were occurring multiple times. This meant increased load on
> the Kerberos server as well as extra log messages in SSSD's DEBUG logs
> (as noticed by Kaushik)
> > Is it a problem with the shipped version?
>
> Yes
> > Can it cause a leak or other issues?
>
> It is technically a leak, but it is a very small amount of memory (one
> additional tevent_timer object for each time we switched from offline
> mode to online mode).
> > Should we add it to errata?
>
> I think that's should be decided by QE. I'm on the fence. The memory
> leak doesn't concern me (since it's small) but the additional load on
> the KDC might be worth the errata.

There should be no additional load on the KDC because the first task
which will renew the TGT will also update the time for the next renewal.
If a second task is started while the first one is still running it will
skip the TGTs which are currently processed because
renew_data->pd == NULL.

bye,
Sumit
> sssd-devel mailing list
> [e-mail]
> https://fedorahosted.org/mailman/listinfo/sssd-devel

[Stephen Gallagher]

On Fri, 2011-05-27 at 13:02 +0200, Sumit Bose wrote:
> Hi,
>
> I confused the online callbacks with tevent timers when adding them to
> the TGT renewal task with the effect that a new renewal timer was added
> every time the system switched back from offline to online. Thanks to
> Kaushik for catching this. The attached patch should fix it.

Ack.

[Stephen Gallagher]

On Thu, 2011-06-02 at 12:53 -0400, Stephen Gallagher wrote:
> On Fri, 2011-05-27 at 13:02 +0200, Sumit Bose wrote:
> > Hi,
> >
> > I confused the online callbacks with tevent timers when adding them to
> > the TGT renewal task with the effect that a new renewal timer was added
> > every time the system switched back from offline to online. Thanks to
> > Kaushik for catching this. The attached patch should fix it.
>
> Ack.

Pushed to master and sssd-1-5.