Add Kudu bindings #444
Add Kudu bindings #444
Conversation
Currently doesn't include unit tests since we don't have mock objects for that.
| System.out.println("Connecting to the masters at " + masterQuorum); | ||
| } | ||
|
|
||
| List<ColumnSchema> columns = new ArrayList<ColumnSchema>(11); |
There was a problem hiding this comment.
There's no guarantee that there will be 10 non-key columns. Can you make this a variable based on the value of fieldcount?
| uses 4 tablets. A good rule of thumb is to use 5 per tablet server. | ||
| * `kudu_table_num_replicas`: The number of replicas that each tablet will have. The default is 3. Should | ||
| only be configured to use 1 instead, for single node tests. | ||
| * `kudu_sync_ops`: If the client should buffer data before sending it. The default is false. Should |
There was a problem hiding this comment.
If this should always be true for the run phase, maybe that should be the default?
There was a problem hiding this comment.
we ask datastore bindings to default to no client-side buffering. I'd rather have this rephrased to not suggest it always be true for the run phase. we do the same to e.g. HBase, Cassandra, etc.
There was a problem hiding this comment.
IIUC, true indicates that there shouldn't be client side buffering, so having it be true for the run phase is correct. I think the default should be true as well.
There was a problem hiding this comment.
Ah yeah my description here is off. True means it won't buffer. I'll make it default to that and change that line.
|
Thanks! One more question, otherwise looks good. |
|
Thanks, @jdcryans! |
Add Kudu bindings
Add Kudu bindings
Currently doesn't include unit tests since we don't have mock objects for that.