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

Allow IMMUTABLE functions in modification commands #108

Open
gmcquillan opened this issue May 5, 2015 · 8 comments
Open

Allow IMMUTABLE functions in modification commands #108

gmcquillan opened this issue May 5, 2015 · 8 comments
Labels
Milestone

Comments

@gmcquillan
Copy link

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
Copy link
Author

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
Copy link
Collaborator

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
Copy link
Collaborator

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
Copy link
Author

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
Copy link
Collaborator

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
Copy link
Author

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
Copy link
Collaborator

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 😁 🏆!

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 jasonmp85 changed the title Support Function Calls for Sharded Modification. Allow IMMUTABLE functions in UPDATE assignment clauses Aug 17, 2015
@jasonmp85 jasonmp85 changed the title Allow IMMUTABLE functions in UPDATE assignment clauses Allow IMMUTABLE functions in modification commands Aug 17, 2015
@jasonmp85
Copy link
Collaborator

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.

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

No branches or pull requests

2 participants