mendicant-original / community

Mendicant University's community website
community.mendicantuniversity.org
20 stars 10 forks source link

Pg advisory lock #79

Closed ptn closed 12 years ago

ptn commented 12 years ago

WIP, looking for opinions on the comments I made inline.

In general, I'm not sure if this is better than the previous approach. Given that an advisory lock has no idea whatsoever about what table, row and column it is locking, I still had to use a separate model to keep track of all that. It's also possible that I completely misunderstood the concept behind this kind of locking - I was tied in a knot for a while there, which is why this took a little longer than expected.

TODOs:

  1. Write a new generator for the Lock model, i.e rails g create_lock_table or similar.
  2. Always capture the result of connection.execute
jordanbyron commented 12 years ago

@ptn I'm open to any suggestions on how we can make this better. I liked the idea of using Postgres's build-in locking mechanisms, but I'm not too crazy that we need to create our on meta table to track additional lock information. Any thoughts on how we could solve this?

ptn commented 12 years ago

I think that if we want to use advisory locks, we can't save neither the table name in integer form nor the row's id in pg_locks.

This document says that:

Integer keys are displayed with the first key in the classid column,
the second key in the objid column, and objsubid equal to 2.
**The actual meaning of the keys is up to the user.**

So if we still used the hack of only using advisory locks within the context of the lock table, we can define classid to be the Lock object's id and objid the owner's id, like so:

connection.execute("SELECT pg_try_advisory_lock(#{self.id}, #{user.id})"

I think it's the best we can do. @ericgj any ideas?

jordanbyron commented 12 years ago

@ptn even with that change, we still are writing to a Lock record, which again doesn't take full advantage of the advisory lock system. Since we need to track who locked the record, I think we should probably go back to just using a Lock table and not bother with postgres's advisory locks. :confounded: Maybe you can salvage your old pull request and pick up from there?

ptn commented 12 years ago

Yeah, I think we should. Boo :-(

There are a few changes that I need to make on the old branch and then I'll send the pull request again.