> > > [1]
> > >
> > > + create_checkpoint_nonce()
> > > + # Delete old nonces.
> > > + checkpoint = find_checkpoint_nonce()
> > > + if checkpoint is None:
> > > + return 0
> > > + return delete_old_nonces(checkpoint)
> > >
> > > Seems odd to create the new nonce before deleting; there is a very
> > > slow race condition there.
> >
> > Where do you see the race condition exactly?
>
> In cleanup_old_nonces():
>
> + create_checkpoint_nonce()
> + # Delete old nonces.
> + checkpoint = find_checkpoint_nonce()
>
> If there's a delay between create_checkpoint_nonce() and
> find_checkpoint_nonce() of more than timestamp_threshold - which I
> grant is *extremely* unlikely - then it'll delete the nonce just
> created.
>
> However, it doesn't matter anyway: by then it'll be time to delete the
> new nonce! So, it's fine to leave it as it is.
That's what I had in mind, we can call that a race, because technically, this is a race but it has no bad consequence whatsoever. Plus it's highly unlikely.
> PostgreSQL does return a count of deleted rows, eliminating the need
> for a separate count, which might be a way to mitigate this. I wonder
> if Django surfaces that in some way?
I tried to see if I could find the 'rowcount' in Django's guts after the query has been performed but I can't so far…
> Another thought: key__endswith probably creates a LIKE '%SUFFIX'
> clause, which can be very inefficient. I suggest talking to Jeroen or
> Stuart Bishop about this. Can this code be changed to use a prefix
> match instead? Can you check that an index on Nonce.key exists, and
> that it supports prefix (or suffix) matches?
I changed that to using prefixes.
But I'm not really concerned about the efficiency here considering that only the *second* run can be a bit inefficient.
> > > [1] checkpoint_ nonce() _nonce( ) old_nonces( checkpoint) old_nonces( ): checkpoint_ nonce() _nonce( ) checkpoint_ nonce() and _nonce( ) of more than timestamp_threshold - which I
> > >
> > > + create_
> > > + # Delete old nonces.
> > > + checkpoint = find_checkpoint
> > > + if checkpoint is None:
> > > + return 0
> > > + return delete_
> > >
> > > Seems odd to create the new nonce before deleting; there is a very
> > > slow race condition there.
> >
> > Where do you see the race condition exactly?
>
> In cleanup_
>
> + create_
> + # Delete old nonces.
> + checkpoint = find_checkpoint
>
> If there's a delay between create_
> find_checkpoint
> grant is *extremely* unlikely - then it'll delete the nonce just
> created.
>
> However, it doesn't matter anyway: by then it'll be time to delete the
> new nonce! So, it's fine to leave it as it is.
That's what I had in mind, we can call that a race, because technically, this is a race but it has no bad consequence whatsoever. Plus it's highly unlikely.
> PostgreSQL does return a count of deleted rows, eliminating the need
> for a separate count, which might be a way to mitigate this. I wonder
> if Django surfaces that in some way?
I don't think Django exposes that. There is a still-open tick for it in Django bugtracker: https:/ /code.djangopro ject.com/ ticket/ 16891.
I tried to see if I could find the 'rowcount' in Django's guts after the query has been performed but I can't so far…
> Another thought: key__endswith probably creates a LIKE '%SUFFIX'
> clause, which can be very inefficient. I suggest talking to Jeroen or
> Stuart Bishop about this. Can this code be changed to use a prefix
> match instead? Can you check that an index on Nonce.key exists, and
> that it supports prefix (or suffix) matches?
I changed that to using prefixes.
But I'm not really concerned about the efficiency here considering that only the *second* run can be a bit inefficient.