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

Allow IMMUTABLE functions in modification commands #108

Open gmcquillan opened 9 years ago

gmcquillan commented 9 years ago

I'm experimenting with using HyperLogLog (HLL) data types in some columns. One problem with these is that they take up quite a lot more space than a BIGINT. pg_shard potentially allays a lot of those issues. The extension that provides the HLL datatype is this one by aggregateknowledge. These two extensions seem complimentary for warehousing purposes.

Surprisingly, these data types work with sharded tables for most types of reads, but not for writes (see below). When I attempt an update like so:

update test_hll_shard set users = users||hll_hash_text('foobar') where date = date('2015-01-08');

I get:

ERROR:  cannot plan sharded modification containing values which are not constants or constant expressions

I can sort of work around this by setting the literal bytes in this field. Which works fine -- but adding HLL values requires a read and a write. Since pg_shard (understandably) doesn't allow for more than a single statement transaction, this leaves my use-case vulnerable to race conditions in multi-writer environments.

Since this function is available on the workers, and is deterministic based on the value of the existing row and the new HLL value to be added, there shouldn't be any issue with dispatching this expression through to the workers.

Is there a hard limitation preventing pg_shard from dispatching modifications for non-constant expressions?

gmcquillan commented 9 years ago

Similarly, as mentioned in the pg_shard-users list, subqueries might be another work around to the function issue, but they are also not supported yet. (e.g. perform the function on data from a different table on the master and insert only constant values into the sharded table).

jasonmp85 commented 9 years ago

Since this function is available on the workers, and is deterministic based on the value of the existing row and the new HLL value to be added, there shouldn't be any issue with dispatching this expression through to the workers.

Are you sure this issue is due to the hll_hash_text function and not the date one? The HLL function is marked as IMMUTABLE, which means it should be folded into a constant before being pushed to the workers…

This might be an argument that the error message should be clarified to say which expression is non-constant.

Is the HLL extension installed on the master? Does it know the details of these functions?

jasonmp85 commented 9 years ago

Is there a hard limitation preventing pg_shard from dispatching modifications for non-constant expressions?

At the moment, yes. pg_shard performs UPDATEs by grabbing a lock for a shard and updating all replicas of that shard. This means there is no first-class replication happening but rather a replica-by-replica execution of the query at hand. If that query had e.g. a now() call, the output might differ among replicas, resulting in data divergence.

So unless we can fold an expression into a constant expression, modifications are unsafe if replication is in play.

gmcquillan commented 9 years ago

To usefully combine or update HLL datastructures, you need to call hll_add, which is not immutable.

I didn't think about replication. That's interesting. I assumed that each shard was responsible for its own replication (streaming WAL logs), not that it was something that pg_shard handled for me (if I'm understanding you correctly).

Thanks for taking the time to correct my misunderstanding.

jasonmp85 commented 9 years ago

Streaming replication replicates the entire database which is incompatible with our use of many small "logical" shards. Because of this we've been keeping an eye on BDR/UDR, but don't have a timeline or even any concrete designs at this point.

We could probably expand the use of functions to those which are STABLE so long as we can ensure the snapshots on each replica are identical when we run the command… We'll definitely take your input into consideration during the next cycle, as we like the HLL extension a lot, too and want it to work well with our software.

gmcquillan commented 9 years ago

Yeah, one really, really nice property of storing data as HLL data types is that mutation is idempotent, which has a nice resiliency in distributed systems.

jasonmp85 commented 9 years ago

Hm… so I just checked hll_add and it is also listed as IMMUTABLE

We find the HLL extension very useful for a number of our customers, so I want to make sure it's working well with pg_shard. I'm curious about the specifics of what you're doing, especially since you've earned yourself the distinction of being the first external party to open a pull request against pg_shard :grin: :trophy:!

Could you shoot me an email at engage at citusdata dot com to have a quick chat about the problem you're working on?

jasonmp85 commented 9 years ago

OK, so I've investigated what's going on here. Previously we had the assumption that IMMUTABLE functions would be collapsed into constants during our call to eval_const_expressions. Because of that, we expected all allowable function calls to have been reduced to constant expressions by the time our planner gets a hold of a query.

This is obviously true when transforming something like 2 + 2 into a constant 4. But if the query were something like UPDATE table SET counter = counter + 1 then the presence of a Var (counter) would keep eval_const_expressions from doing its thing (rightly so).

I think we can relax this check by replacing it with a call to contain_mutable_functions instead, allowing immutable functions to be pushed down to remote nodes even if they can't be fully reduced at the master.

Because UPDATEs execute against a single shard and its replicas, and because UPDATE is one-at-a-time (due to locking), it's safe to do this pushdown.

We'll look into this during a future cycle.