Updated schema for emails.participant to refer to participants.id

[sambuddhabasu]

Changes made in this PR:

• Added sql/branch.sql. The emails.participant now refers to participants.id rather than participants.username.

@whit537 , the tests fail now as the schema is now changed. Please let me know if the schema change looks good and I will move ahead with fixing the tests.

[chadwhitacre]

cc: @rohitpaulk @rorepo

[chadwhitacre]

What's the effect of the participant::bigint cast? Does this properly convert the values in the emails.participant column?

[sambuddhabasu]

The participants.id datatype is bigserial. So, the datatype of emails.participant need to match with this. In order to do so, we need to first convert it into bigint and then create a sequence for the column. We can read more about serial type from here.

[chadwhitacre]

What happens to the old data in the column with this statement?

[rohitpaulk]

I think this can be done in a better fashion. If we try to couple application code changes and database migrations (such that they both have to be made at once), we're going to require a certain amount of downtime - And if we end up with errors in the migration code, that could be a lot of downtime.

If we incrementally make changes to the application and schema, such that they don't trample over each other - this whole operation would be much safer. Here's how I think this can be done -

1. MIGRATION - Add a participant_id column to emails, along with the foreign key constraint. Let's start with NULL values by default, we'll fill in values over time.
2. CODE CHANGE - Modify code such that wherever emails.participant is written to, it writes to emails.participant_id too.
3. MIGRATION - Here's the interesting part. Now we have all the time in the world to work on filling in the NULL emails.participant_id values. It's not going to affect the app at all, since the app is still relying on emails.participant for reads.
4. CODE CHANGE - Once we're confident that we've filled in email.participant_id accurately, the application code can be modified to read from email.participant_id, and write to only email.participant_id.
5. MIGRATION - email.participant isn't being used anymore, safe to remove.

Does that make sense, @sammyshj?

[rohitpaulk]

Here's some reading material that might help:

https://gocardless.com/blog/zero-downtime-postgres-migrations-the-hard-parts/
https://blog.rainforestqa.com/2014-06-27-zero-downtime-database-migrations/
http://blog.honeybadger.io/zero-downtime-migrations-of-large-databases-using-rails-postgres-and-redis/

[chadwhitacre]

Ping @sammyshj. Are you stuck here? Can we help you get unstuck? :-)

[rohitpaulk]

@sammyshj - I'm closing this in favour of #3893 and the following PRs. We've still got more tables to cover on #835, feel free to take them up :)

[techtonik]

I think this can be done in a better fashion.

@rohitpaulk is the idea of speeding up the migrations still live to deserve a new ticket?

[rohitpaulk]

is the idea of speeding up the migrations still live to deserve a new ticket?

I think you're getting confused with two different migrations here, @techtonik. Seemed like you were talking about #3894?

To clear things up -

#3894 deals with speeding up the tip migration script that we run every Payday. When users who previously had tips on Gratipay create Teams - we migrate their tips over (and create subscriptions, in Gratipay 2.0 terminology)

#835 is what this PR is related to, it deals with replacing username (and using id instead) as a primary key on the participants table.

[techtonik]

I see that #835 is already implemented in 3 PRs. Unused participant in email table can be used to track original nickname (in case account is hijacked), but that's better be left for another ticket.

[aandis]

@techtonik the unused emails.participant is already dropped in #3898.

[techtonik]

@aandis yea, I saw that. Just a thought that now data dump is not as convenient to for human consumption as it was.