citusdata / pg_shard

ATTENTION: pg_shard is superseded by Citus, its more powerful replacement
https://github.com/citusdata/citus
GNU Lesser General Public License v3.0
1.06k stars 63 forks source link

Modify Citus metadata sync to be idempotent #88

Closed jasonmp85 closed 9 years ago

jasonmp85 commented 9 years ago

A customer noticed this function could not be called more than once without error. Now calling it more than once has the same effect as calling it once. In addition, if a shard placement's state has changed since the most recent call, an additional call will update the placement state in CitusDB's view.

Added a unit test to verify these cases.

For an explanation of the algorithm in use here, read the Bulk upsert with lock section in this Stack Overflow answer.

Code review tasks

onderkalaci commented 9 years ago

@jasonmp85 For style issues of SQL I am not much experienced. Also, we don't have a documentation for SQL styling. I only added commented for the style that Ozgun already suggested. In general, I have a comment. Compared to CitusDB SQL examples, we should maybe not start a new line on each AND clause in the WHERE statements. For example, instead of

        WHERE  shardid IS NULL
               AND relation_id = table_relation_id;

we may consider the following to make it similar to CitusDB:

        WHERE  shardid IS NULL  AND relation_id = table_relation_id;

But, I am not sure about that, so I didn't comment on this cases. Just write here to discuss.

jasonmp85 commented 9 years ago

Since I want to get this wrapped up, I'm assuming after these points are addressed, this is a :shipit:… if not, can duly gripe at me and I'll fix any other issues in another branch :grimacing:.