Bug in take_over

[zbynekwinkler]

There is a bug in gittip/models/_mixin_elsewhere.py#L119 that results in two entries in the tip table with the same (tipper, tippee, mtime) but different amount. When getting current tips with

select distinct on(tipper, tippee)...
order by (tipper, tippee, mtime desc)

the result in nondeterministic. This is based on the documentation for DISTINCT:

Note that the "first row" of each set is unpredictable unless ORDER BY is used to ensure that the desired row appears first.

A possible way to get into this situation is:

alice = TwitterAccount(1, dict(screen_name='alice'))
bob   = TwitterAccount(2, dict(screen_name='bob'))
alice_participant = alice.opt_in('alice')[0].participant
bob_participant = bob.opt_in('bob')[0].participant
alice_participant.set_tip_to('bob', '1.00')
bob_participant.take_over(alice, have_confirmation=True)

a = self.db.one("""
        SELECT count(*) 
          FROM (
                  SELECT * FROM tips 
                  EXCEPT 
                  SELECT DISTINCT ON(tipper, tippee, mtime) * 
                    FROM tips 
                ORDER BY tipper, tippee, mtime
                ) AS foo
             """)
assert a == 0

That is when there is a outstanding tip from 'alice' to 'bob' and 'bob' takes over 'alice'. I've found it by accident when working on #1549.

[zbynekwinkler]

To reflect the situation that 'alice' is going to be removed we need to take care of all tips she gives or receives.

• take all tips where tippee='alice' and tipper<>'bob' and replace them with tips where tippee='bob'
• take all tips where tipper='alice' and tippee<>'bob' and replace them with tips where tipper='bob'

I believe that is what gittip/models/_mixin_elsewhere.py#L168 and gittip/models/_mixin_elsewhere.py#L185 is trying to do however I have no idea how 😢

I have reworked the two queries into the following form:

# creates new set of merged tips with the 'live' account as receiver/giver
# * first condition includes all tips to/from the two users
#   to merge them together with sum() and group by
# * second condition filters out tips between the two accounts involved
#   to remove self tipping

            INSERT INTO tips (ctime, tipper, tippee, amount)

                 SELECT min(ctime), tipper, %(live)s AS tippee, sum(amount)
                   FROM (   SELECT DISTINCT ON (tipper, tippee)
                                   ctime, tipper, tippee, amount
                              FROM tips
                          ORDER BY tipper, tippee, mtime DESC
                         ) AS unique_tips
                  WHERE (tippee = %(dead)s OR tippee = %(live)s)
                AND NOT (tipper = %(dead)s OR tipper = %(live)s)
               GROUP BY tipper

            INSERT INTO tips (ctime, tipper, tippee, amount)

                 SELECT min(ctime), %(live)s AS tipper, tippee, sum(amount)
                   FROM (   SELECT DISTINCT ON (tipper, tippee)
                                   ctime, tipper, tippee, amount
                              FROM tips
                          ORDER BY tipper, tippee, mtime DESC
                         ) AS unique_tips
                  WHERE (tipper = %(dead)s OR tipper = %(live)s)
                AND NOT (tippee = %(dead)s OR tippee = %(live)s)
               GROUP BY tippee

Which passes all the tests including the new one above.

[zbynekwinkler]

Payday processes orphans. Fortunately the tips that are left behind are between two accounts that belong to the same person. However we are risking that other bugs will not be so harmless 😉

Which made me check:

SELECT foo.tipper, t.timestamp, t.amount 
                  FROM (
                          SELECT * FROM tips 
                          EXCEPT 
                          SELECT DISTINCT ON(tipper, tippee, mtime) * 
                            FROM tips 
                        ORDER BY tipper, tippee, mtime
                        ) AS foo
  JOIN transfers t on t.tipper = foo.tipper
  JOIN absorptions a on a.archived_as = foo.tipper
 WHERE t.timestamp > a.timestamp;

And that unfortunately returns 4 transactions that were done after the original tipper was absorbed. Fortunately to the user that was the absorber. But I still think we should be more conservative/careful in payday and not rely on the fact that orphans have the tips cleared.

[zbynekwinkler]

This will show the conflicting tips:

SELECT * FROM tips t
WHERE (t.tipper, t.tippee, t.mtime) IN 
      ( SELECT tipper, tippee, mtime
          FROM
             (
                SELECT * FROM tips 
                EXCEPT 
                SELECT DISTINCT ON(tipper, tippee, mtime) * 
                  FROM tips 
              ORDER BY tipper, tippee, mtime
              ) AS foo
       );

and this will attempt to delete them:

DELETE FROM tips t
WHERE (t.tipper, t.tippee, t.mtime) IN 
      ( SELECT tipper, tippee, mtime
          FROM
             (
                SELECT * FROM tips 
                EXCEPT 
                SELECT DISTINCT ON(tipper, tippee, mtime) * 
                  FROM tips 
              ORDER BY tipper, tippee, mtime
              ) AS foo
       )
   AND t.amount <> 0
RETURNING *;

but it will miss two conflicts because all tips are zero. Which is even stranger because I have no idea how that could have happened and the above mentioned test does not cover it 😩

[zbynekwinkler]

Ok, it happens when there was a tip from 'alice' to 'bob', 'alice' cleared the 'tip' and was taken over by 'bob'. Hmm. That makes me think that taking over a participant might take over even the tips that are currently zero. Oh yes, it does.

[chadwhitacre]

Awesome debugging. :-)

[chadwhitacre]

[...] is trying to do however I have no idea how 😢

You noticed the docstring for take_over, yes?

[zbynekwinkler]

You noticed the docstring for take_over, yes?

Yes, but I couldn't connect the explanation with the WHERE clause in those queries.

[chadwhitacre]

I've repaired the db per #1704 (comment) and IRC.

!m @zwn.

[chadwhitacre]

The fix for this is in #1706.