impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Thomas Tauber-Marshall (Code Review)" <>
Subject [Impala-ASF-CR] IMPALA-4591: Bound Kudu client error mem usage
Date Wed, 08 Nov 2017 19:40:02 GMT
Thomas Tauber-Marshall has posted comments on this change. (

Change subject: IMPALA-4591: Bound Kudu client error mem usage

Patch Set 2:

File be/src/exec/kudu-table-sink.h:
PS1, Line 105: consumed 
> nit: "consumed" to be consistent with the memtracker terminology.
PS1, Line 106: client_tracked
> Maybe client_tracked_bytes_ to make it clearer that the unit is bytes and i
File be/src/exec/
PS1, Line 52: 
> Was this flag documented? Just wondering if we should consider what happens
No, its not documented, and of course it specifically says that it may be changed/removed.

Certainly happy to consider other options if you think removing the flag is too disruptive,
eg. the error buffer size could be calculated as the difference between "kudu_sink_mem_required"
and "kudu_mutation_buffer_size", that just seemed a little complicated.
PS1, Line 39: DEFINE_int32(kudu_mutation_buffer_size, DEFAULT_KUDU_MUTATION_BUFFER_SIZE,
> Just throwing out ideas here, but did we think about pros/cons of making th
I think that's a reasonable idea, though like you say these probably don't need to be modified
very often.

At the least, I think it makes sense to get this in and file a JIRA for followup.
PS1, Line 124:   int64_t required_mem = FLAGS_kudu_mutation_buffer_size + error_buffer_size;
> Is this equivalent to the following?
PS1, Line 132:       state->exec_env()->GetKuduClient(table_desc_->kudu_master_addresses(),
> nit: long line.
File tests/custom_cluster/
PS1, Line 66:   @CustomClusterTestSuite.with_args(impalad_args="-kudu_error_buffer_size=1024")
> It might be faster to make this a regular query test but insert more data s
It takes a very large number of errors to hit the default limit (>10m "Key already present"
errors), so I don't think it ends up being any faster.
PS1, Line 74: presen
> present

To view, visit
To unsubscribe, visit

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I186ddb3f3b5865e08f17dba57cf6640591d06b14
Gerrit-Change-Number: 8464
Gerrit-PatchSet: 2
Gerrit-Owner: Thomas Tauber-Marshall <>
Gerrit-Reviewer: Thomas Tauber-Marshall <>
Gerrit-Reviewer: Tim Armstrong <>
Gerrit-Comment-Date: Wed, 08 Nov 2017 19:40:02 +0000
Gerrit-HasComments: Yes

  • Unnamed multipart/alternative (inline, 8-Bit, 0 bytes)
View raw message