Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

most_recent column prevents HOT updates #206

Open
MSch opened this issue Mar 10, 2016 · 3 comments
Open

most_recent column prevents HOT updates #206

MSch opened this issue Mar 10, 2016 · 3 comments

Comments

@MSch
Copy link

MSch commented Mar 10, 2016

https://github.com/postgres/postgres/blob/master/src/backend/access/heap/README.HOT

The statesman migration creates an index on the most_recent column. This prevents HOT updates on the transitions table.

The first solution that comes to my mind is to get rid of most_recent and instead have a last_sort_key on the parent model, then start every transition with an optimistic update of last_sort_key, which will lock the row for the rest of the transaction, which will prevent concurrent transitions, ensuring their unique ordering.

Incrementing, locking and retrieving all columns could even be done in a single statement: UPDATE <model> SET last_sort_key = last_sort_key + 1 RETURNING *.

@greysteil
Copy link
Contributor

Thanks for the issue @MSch. I'd rather avoid adding any columns to the parent model if at all possible - it's generally not the philosophy for Statesman.

@Sinjo - can you have a think about this one? Would value your opinion on it above my own!

@MSch
Copy link
Author

MSch commented Mar 10, 2016

@greysteil I understand and I agree that it makes sense to avoid putting requirements on the parent model. The architecture of statesman is why we want to use it :)

I don't want to do too much guessing about performance, the tradeoff I see is between a) having to maintain an index on the transition table (which then furthermore prevents HOT updates) vs. b) having to lock the parent (and requiring a column to exist)

Maybe you could make the last_sort_key column on the parent model optional, and if it's not there just lock the parent model and perform the transition then. Same guarantees, since no transition can happen while the parent is locked and you get HOT updates without hard requirements on the parent model.

@Sinjo
Copy link
Contributor

Sinjo commented May 14, 2017

So first off, sorry that there was literally a year of delay responding to this. I've not been involved in the code that uses Statesman for a while, so it's not been on my radar. The good news is that in that year I've learned a lot more about Postgres, including HOT and what it does.

This one's a tough call. We're generally keen to keep Statesman-related fields on the transition model. Is this something you've concretely seen performance problems with?

There's a semi-related PR (#228, cc @hmac who's been involved in that) that proposes locking the parent model due to deadlocks. On pure performance grounds, if that goes through then this issue becomes a question of code/database organisation rather than a performance comparison, as we'd be locking the parent model anyway.

Still, my default here is not to change our current approach. We need to see some pretty compelling performance benchmarks before we do.

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

Successfully merging a pull request may close this issue.

3 participants