Skip to content
This repository has been archived by the owner on Feb 8, 2018. It is now read-only.

Clean up exchanges #4442

Closed
6 of 15 tasks
kaguillera opened this issue May 3, 2017 · 46 comments
Closed
6 of 15 tasks

Clean up exchanges #4442

kaguillera opened this issue May 3, 2017 · 46 comments

Comments

@kaguillera
Copy link
Contributor

kaguillera commented May 3, 2017


✈️ This is the flight deck for the Clean up exchanges project. ✈️


Due to bad schema design we have missing values for some fields in our exchanges and exchange_routes tables. We use either unknown or NULL to represent this missing info (we started using unknown in #3975 in order to make querying easier).

table field value
exchanges ref NULL
exchanges status unknown
exchange_routes network unknown

Todo

Original

Confused. So far.

  • There is no payday log for payday 0, payday 1, payday 2
  • The payday log for payday run on 2012-06-22 is no where to be found
  • There are exchange records in the exchange table for all of the above paydays assuming that
    - Payday 0 was run on 2012-06-01
    - Payday 1 was run on 2012-06-08
    - Payday 2 was run on 2012-06-15 which according to gittip-3.log (payday logs it was)
    - Payday 3 was run on 2012-06-22 but no log exist but there are records of exchanges in the exchanges table
    - Payday 4 was run on 2012-06-29 but is actually recorded in gittip-4.log
  • Even though there are exchange records for all the paydays mentioned above not all the records for all the transactions exist in the exchanges table. That is to say for example in payday logs for the payday run on 2012-06-15 ( recorded in gittp-3.log) 18 people were charged ( I am assuming via samurai since that was the vender we were using at that time) but only 3 exchanges were recorded. This possibly happened because of a database error since I am seeing some error statements at the end of the log file.

✈️ This is the flight deck for the Clean up exchanges project. ✈️


@kaguillera
Copy link
Contributor Author

kaguillera commented May 4, 2017

Turns out that we started using Stripe from payday 3 run on 2012-06-22 of which there is no log file for that payday.
Thus all exchanges recorded before 2012-06-22 the routes (network) for these set of records should be samurai. After that most of the routes should be Stripe.

@chadwhitacre
Copy link
Contributor

Ref: .#3912

@kaguillera
Copy link
Contributor Author

kaguillera commented May 4, 2017

The exchange records for payday 0, payday 1 and payday 2 run on 2012-06-01, 2012-06-08 and 2012-06-15 respectively. There is just one record that is out of place.
I am thinking that for those initial 16 exchanges and 13 exchange_route we can manually update them. We know that the network is Samurai...but what do we put for the address and ref? Do we have any way of getting it or should be generate a fake one like in the script? I suggest that we generate a fake one as the backfill.py script does.
Also based on the Samurai logs they succeeded so the status should be succeeded

@kaguillera
Copy link
Contributor Author

For the Stripe exchanges
exchanges.route = exchange_routes.id
exchanges.status = stripe transaction status
exchanges.ref = transaction_id from stripe (e.g. ch_7h*********cP)

exchange_routes.network = stripe
exhcange_routes.address = stripe customer_id (e.g. cus_1d3*********4z)

Were have all of the necessary information in the logs directory under misc/3912/year/month/stripe.
So what went wrong why is the network unknown and the address n/a for some some exchange routes.

@kaguillera
Copy link
Contributor Author

kaguillera commented May 4, 2017

In the exchange_routes table I have there are not stripe network value is it possible that all the unknown values are actually stripe exchange_routes?

How did I check this?

@kaguillera
Copy link
Contributor Author

Turns out that the unknown could also be samurai but why is this happening.

@kaguillera
Copy link
Contributor Author

kaguillera commented May 4, 2017

And also balanced-ba exchange_route. what went wrong. Time to check the code I guess.
My though is if I can identify in the code where we set the network field to unknown I should be able to figure out what is going wrong.

@kaguillera
Copy link
Contributor Author

Additionally I want look more closely at the error statements at the end of the gittip-3.log file

@kaguillera
Copy link
Contributor Author

There seem to be no relation between the errors at the end of gittip-3.log and the three successfully recorded records in the exchanges table. I was hoping that the users that showed up in the errors were the transactions that were not recorded but there is a user listed in the error that was successfully recorded in the exchanges table.
It could be that the transactions recorded were the only successful ones. The log gittip-3.log does not specify if the transaction was successful or not.
Moving on to look at the code that insert or updated the route and ref values in the exchanges table.

@kaguillera
Copy link
Contributor Author

Not finding where we introduced the exchange_routes. 😱

@kaguillera
Copy link
Contributor Author

kaguillera commented May 19, 2017

So IRL @whit537 just reminded me that I made the unknown when I was redoing the dashboard reconciliation.spt page #3975. 😱 😭

@kaguillera
Copy link
Contributor Author

Ok so here is the plan. Correct the unknown exchange_routes and then make backfill the references.

How do I correct the exchange_routes?
I am thinking I could use the payday logs along with the exchanges table to match participants the transactions and thus networks. This should work since pre gittp-8.log we only used two payment_networks namely samurai and Stripe. and after that we specified in the logs which payment_network we were charging the participant on. Thus from that I can make routes for the participants and update the exchanges with that route_id
Theoretically!

@kaguillera
Copy link
Contributor Author

kaguillera commented May 24, 2017

Current payment_net values

    network
--------------
 balanced-ba
 balanced-cc
 paypal
 bitcoin
 braintree-cc
 cash
 transferwise
 dwolla
 unknown
(9 rows)

We will need to add samurai and stripe

@kaguillera
Copy link
Contributor Author

#3807 & #2779 - Back filled the exchanges and created routes for Balanced transactions when we were refunding and/or Balanced shutdown.

NB: In the payday log files we started recording success in the on the 4th payday gittip-4.log

This match-stripe.py is a script to match Stripe transactions with exchanges to produce the input file for the backfill.py script.

But the issue here is

The problem we're up against is ... usernames as primary key (#835). 😊

The way we log username changes, we don't record the old username, only the new one. This makes it impossible to trace back to what username a participant had at a particular point in time. We do have old database snapshots we could look at, but we didn't add user id until late enough that an old snapshot wouldn't help us map old username to current user id anyway.

But I think that we can backfill what we can and figure out how to resolve this issue after. There should only be a small amount of these cases.

For now we can assume that all recorded transaction/exchanges with a null or unknown were successful cf (#3912 (comment))

After we have made finished backfilling we can always use the payday log files for payday #4 and onward to correct the status if necessary.

@kaguillera
Copy link
Contributor Author

NB The previous plan

match-balanced.py

Issues with data corruption that I need to research next before we go ahead...tomorrow

@kaguillera
Copy link
Contributor Author

exchanges record

id timestamp amount fee participant status route
327 2012-07-27 20:54:01.856078+00 9.32 0.68 bekishore succeeded 8219

exchange_route record for the above exchange

id participant network address is_deleted
8219 9815 braintree-cc CC3xxxxxxxxxxxxxxxlw f

Note the timestamp date of the exchange. We did not start using braintree until some time in 2015 after the Balanced shutdown s clearly this is wrong. Additionally in the payday log for that date it it records the following...
pid-24223 thread-140735091989696 (MainThread) Charging bekishore 1000 cents ($1.00 + $0.36 fee = $1.36, round ed up to $10.00) on Balanced ... succeeded.

Not sure how that happened. The point I am making is that besides having to backfill exchanges and exchange_routes we will have to do an audit/reconciliation/corrects possibly using the payday logs. I think that we should do this in a separate PR after we have done the backfill

@kaguillera
Copy link
Contributor Author

So my conclusion after researching is that we are on the right track and we should continue implementing the back fill the way we were with a few changes. The main change is that we create new exchange_routes for exchanges where the exchange_route network field is unknown

Thus the next step is to add samurai and stripe to the payment_net ENUM and then follow the steps here

@kaguillera
Copy link
Contributor Author

Copied from here

How to Backfill Exchanges

For each month in gratipay/finances#3:

  1. generate input CSVs for all vendors that we started using in the given month; for each vendor:
    1. export CSVs for all data for the given vendor—these are our raw input CSVs
    2. write one-off scripts to split up raw input CSVs into one CSV per month in the 3912 directory in the logs repo (backfill branch; prefix with _ to be ignored by backfill.py)—these are the input CSVs
  2. write a match-{foo}.py script to generate 3912/YYYY/MM/foo files (no extension! {foo} matches a payment network in the ENUM in the database)—these are our match CSVs
  3. run backfill.py to backfill all exchanges and exchange_routes needed for the given month based on the match CSVs
  4. reconcile the books for the given month

@kaguillera
Copy link
Contributor Author

Based on the current structure of the backfill branch I believe we have to do step 2 for the _balanced files.

Here goes...

@chadwhitacre chadwhitacre changed the title Research issues with backfilling remaining exchanges Clean up exchanges and exchange_routes Jun 2, 2017
@chadwhitacre
Copy link
Contributor

We plugged one ref leak in #4361 but now it looks like we have another, in MassPay.

Do any of the other three still leak?

@chadwhitacre
Copy link
Contributor

Are we leaking NULL routes?

@chadwhitacre
Copy link
Contributor

No! It has a not null constraint and also does not accept unknown.

@chadwhitacre
Copy link
Contributor

chadwhitacre commented Jun 2, 2017

status and network both have a not null constraint, but do accept unknown.

Are we leaking status?

@chadwhitacre
Copy link
Contributor

chadwhitacre commented Jun 2, 2017

Actually, back up: we've already backfilled all exchanges with a route, so we can remove that as a direct concern. Instead the pertinent field is network ... and address!

gratipay::DATABASE=> select count(*) from exchange_routes where network = 'unknown';
┌───────┐
│ count │
├───────┤
│  1626 │
└───────┘
(1 row)

gratipay::DATABASE=> select count(*) from exchange_routes where address = 'n/a';
┌───────┐
│ count │
├───────┤
│  1626 │
└───────┘
(1 row)

gratipay::DATABASE=> select count(*) from exchange_routes where network = 'unknown' and address = 'n/a';
┌───────┐
│ count │
├───────┤
│  1626 │
└───────┘
(1 row)

@chadwhitacre
Copy link
Contributor

Based on a review of ...

gratipay::DATABASE=> select id, network from exchange_routes order by id desc limit 1000;

... I believe we're no longer leaking network (or address). But!

What about potentially erroneous networks? I.e., braintree-cc per #3912 (comment)#4342 (comment).

@chadwhitacre chadwhitacre changed the title Clean up exchanges and exchange_routes Clean up exchanges Jun 2, 2017
@chadwhitacre
Copy link
Contributor

Here's the end of the network leak:

gratipay::DATABASE=> select id, network, address != 'n/a' have_address from exchange_routes order by id desc offset 488 limit 20;
┌───────┬──────────────┬──────────────┐
│  id   │   network    │ have_address │
├───────┼──────────────┼──────────────┤
│ 14384 │ bitcoin      │ t            │
│ 14383 │ paypal       │ t            │
│ 14382 │ paypal       │ t            │
│ 14381 │ paypal       │ t            │
│ 14380 │ braintree-cc │ t            │
│ 14379 │ braintree-cc │ t            │
│ 14378 │ bitcoin      │ t            │
│ 14377 │ bitcoin      │ t            │
│ 14376 │ paypal       │ t            │
│ 14375 │ bitcoin      │ t            │
│ 14374 │ unknown      │ f            │
│ 14373 │ unknown      │ f            │
│ 14372 │ unknown      │ f            │
│ 14371 │ unknown      │ f            │
│ 14370 │ unknown      │ f            │
│ 14369 │ unknown      │ f            │
│ 14368 │ unknown      │ f            │
│ 14367 │ unknown      │ f            │
│ 14366 │ unknown      │ f            │
│ 14365 │ unknown      │ f            │
└───────┴──────────────┴──────────────┘
(20 rows)

and here's the end of the status leak (waaay back!):

gratipay::DATABASE=> select timestamp, status from exchanges where status='unknown' order by timestamp desc limit 10;
┌───────────────────────────────┬─────────┐
│           timestamp           │ status  │
├───────────────────────────────┼─────────┤
│ 2014-09-19 20:28:57.098478+00 │ unknown │
│ 2014-09-18 19:39:30.573582+00 │ unknown │
│ 2014-09-18 18:41:58.481287+00 │ unknown │
│ 2014-09-17 19:11:46.682704+00 │ unknown │
│ 2014-09-11 15:13:11.663196+00 │ unknown │
│ 2014-09-11 15:13:10.232116+00 │ unknown │
│ 2014-09-11 15:13:08.634617+00 │ unknown │
│ 2014-09-11 15:13:07.118274+00 │ unknown │
│ 2014-09-11 15:13:03.453589+00 │ unknown │
│ 2014-09-11 15:13:01.812733+00 │ unknown │
└───────────────────────────────┴─────────┘
(10 rows)

gratipay::DATABASE=>

@chadwhitacre
Copy link
Contributor

Yeah, this doesn't look right.

Braintree

gratipay::DATABASE=> select substring(date_trunc('year', timestamp)::text from 0 for 5) as year, count(timestamp) from exchanges e join exchange_routes er on e.route = er.id where network='braintree-cc' group by date_trunc('year', timestamp) order by year;
┌──────┬───────┐
│ year │ count │
├──────┼───────┤
│ 2012 │   467 │
│ 2013 │  5115 │
│ 2014 │ 14362 │
│ 2015 │  8576 │
│ 2016 │  2658 │
│ 2017 │  1075 │
└──────┴───────┘
(6 rows)

Balanced

gratipay::DATABASE=> select substring(date_trunc('year', timestamp)::text from 0 for 5) as year, count(timestamp) from exchanges e join exchange_routes er on e.route = er.id where network='balanced-cc' group by date_trunc('year', timestamp) order by year;
┌──────┬───────┐
│ year │ count │
├──────┼───────┤
│ 2012 │   377 │
│ 2013 │  2172 │
│ 2014 │  4269 │
│ 2015 │   600 │
│ 2016 │    63 │
└──────┴───────┘
(5 rows)

gratipay::DATABASE=>

@chadwhitacre
Copy link
Contributor

chadwhitacre commented Jun 2, 2017

As expected, there is no -i braintree in the payday logs until #3486.

@chadwhitacre
Copy link
Contributor

chadwhitacre commented Jun 2, 2017

But I do find braintree files under misc/308/raw/balanced/refund ... 🤔

@chadwhitacre
Copy link
Contributor

The first transactions I find in the Braintree dashboard are on May 29, 2015, which is #3486. See esp. #3486 (comment):

screen shot 2015-05-29 at 2 27 46 am

@chadwhitacre
Copy link
Contributor

So at this point my best hunch is that I made a mistake during gratipay/inside.gratipay.com#308 and conflated balanced and braintree.

@chadwhitacre
Copy link
Contributor

Any further evidence to support that hypothesis?

@chadwhitacre
Copy link
Contributor

Alright, what the heck were we doing on 308 and how did we end up with those logfiles?

@chadwhitacre
Copy link
Contributor

chadwhitacre commented Jun 2, 2017

Oh! Was it related to Balanced accounts that we migrated to Braintree?

@chadwhitacre
Copy link
Contributor

Do we still have the scripts that produced these logfiles?

@chadwhitacre
Copy link
Contributor

😕

/Users/whit537/personal/gratipay/logs/misc/308
total 88
drwxr-xr-x  13 whit537  staff    442 Jun  2 15:38 ./
drwxr-xr-x   9 whit537  staff    306 Mar 17  2016 ../
-rw-r--r--   1 whit537  staff   1342 Sep 30  2015 ally.csv
-rw-r--r--   1 whit537  staff   5769 Sep 30  2015 balanced.csv
-rw-r--r--   1 whit537  staff   4467 Sep 30  2015 balanced.monthly.csv
-rw-r-----@  1 whit537  staff   4555 Sep 22  2015 braintree.csv
-rw-r--r--   1 whit537  staff    136 Sep 23  2015 citizens-checking.csv
-rw-r--r--   1 whit537  staff    140 Sep 23  2015 citizens-money-market.csv
-rw-r--r--   1 whit537  staff  14068 Sep 30  2015 coinbase.csv
-rw-r--r--   1 whit537  staff   4307 Sep 30  2015 paypal.0.csv
-rw-r--r--   1 whit537  staff  11298 Sep 30  2015 paypal.1.csv
-rw-r--r--   1 whit537  staff  13524 Sep 30  2015 paypal.2.csv
drwxr-xr-x  14 whit537  staff    476 Jan  6  2016 raw/
[gratipay] $ ack -i --type=python braintree
[gratipay] $

@chadwhitacre
Copy link
Contributor

Here's the migration: #3391.

@chadwhitacre
Copy link
Contributor

Prrrooommmiisssiiingggg

@chadwhitacre
Copy link
Contributor

Notes from the bus ride ...

Did I overload gratipay/inside.gratipay.com#308 with refunds? What was the ticket for refunds?

All of the braintree.* files under gratipay/inside.gratipay.com#308 that have dates in them (only post-back.log doesn't) have only 2015 things for Braintree.

How about looking at old database backups to determine when these braintree-cc routes were entered?

Search for braintree-cc on GitHub? Can I search my gists for possible script treatment?

What are the patterns in how these routes were set? What are the relative ids of these routes? Can we use that to infer a date range or anything else about the creation or updating of these routes?

Huh! I ran a query to see the year of first exchange for these routes, and I'm seeing it only sparsely populated! Does NULL mean that there was an exchange route that was never used?

        SELECT er.id
             , username
             , address
             , (SELECT substring(timestamp::text from 0 for 5)
                  FROM exchanges
                 WHERE route=er.id
              ORDER BY timestamp asc
                 LIMIT 1
               ) AS year
          FROM exchange_routes er
          JOIN participants p
            ON p.id= er.participant
         WHERE network='braintree-cc'
      ORDER BY er.id asc;

Or are these routes that should be linked up to transactions but aren't yet?

Many of the addresses look like they were migrated from Balanced (CCdeadbeef), but there are plenty using native Braintree-format addresses as well.

These addresses in Balanced format—do we have a route for Balanced and a route for Braintree for the same address?

@kaguillera
Copy link
Contributor Author

So an update for this is the PR under review to take care of plugging last remaining leak: ref under MassPay and add ref to record an exchange via #4518

@chadwhitacre
Copy link
Contributor

Per IRL convo, we're getting to the point where I am going to need to be more involved in this ticket in order to continue making progress. Goal is to land #4518 asap, and then @kaguillera will work on other tickets while I am out traveling for the next couple months. We will revisit in the future.

@chadwhitacre
Copy link
Contributor

I have a collection of old backups. Idea: crawl them and extract usernames over time. Use claimed_time to approximate a unique identifier prior to when we added id.

@kaguillera
Copy link
Contributor Author

Sound good to me...but it will be tedious

@chadwhitacre
Copy link
Contributor

@kaguillera and I talked IRL and are thinking that we should postpone this until we have revenue growth again, since without revenue growth we won't survive very long and backfilling exchanges won't matter.

Well, but ... we should at least plug all leaks. The one remaining is the corrupt network issue (ticket description). What's up with that? Is it truly corrupt and are we continuing to introduce further corruption?

@chadwhitacre
Copy link
Contributor

Re: corruption ... maybe due to overeager matching on address which largely persisted across the migration from Balanced to Braintree?

@chadwhitacre
Copy link
Contributor

More apparent corruption with what route is linked to exchanges: gratipay/inside.gratipay.com#1164 (comment).

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants