[Impala-ASF-CR] IMPALA-4478: Initial Kudu client mem tracking for sink
Internal Jenkins has posted comments on this change. Change subject: IMPALA-4478: Initial Kudu client mem tracking for sink .. Patch Set 2: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/5152 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I47f17a81e4362ab490019382fedc66c25f07080a Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Matthew JacobsGerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Internal Jenkins Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4478: Initial Kudu client mem tracking for sink
Internal Jenkins has submitted this change and it was merged. Change subject: IMPALA-4478: Initial Kudu client mem tracking for sink .. IMPALA-4478: Initial Kudu client mem tracking for sink The Kudu client allocates memory which is not tracked by Impala. There are several sources, but the most significant is the memory allocated by the KuduSession on the write path. This can be >100MB, so it is important to track to avoid OOM. Moving forward, we should have a better way to track Kudu client memory, but for now we must at least handle this potentially problematic case. This changes the KuduTableSink to consume 200MB which should be enough for the 100MB write mutation buffer as well as 100MB worth of errors buffered in the client before Impala takes ownership of them (and deletes them). This is left as a flag because it may turn out to be too high for some users and too low for others. When we have better support from Kudu (including KUDU-1752), we should simplify this. TODO: Handle DML w/ small or known resource requirements (e.g. VALUES specified or query has LIMIT) specially to avoid over-consumption. Testing: Have verified acceptable behavior in the stress test with a simple workload containing DML statements of moderate cardinality. Change-Id: I47f17a81e4362ab490019382fedc66c25f07080a Reviewed-on: http://gerrit.cloudera.org:8080/5152 Reviewed-by: Matthew JacobsReviewed-by: Dan Hecht Tested-by: Internal Jenkins --- M be/src/exec/kudu-table-sink.cc 1 file changed, 37 insertions(+), 3 deletions(-) Approvals: Matthew Jacobs: Looks good to me, approved Internal Jenkins: Verified Dan Hecht: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/5152 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I47f17a81e4362ab490019382fedc66c25f07080a Gerrit-PatchSet: 3 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Matthew Jacobs Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Internal Jenkins Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-4478: Initial Kudu client mem tracking for sink
Dan Hecht has posted comments on this change. Change subject: IMPALA-4478: Initial Kudu client mem tracking for sink .. Patch Set 2: Code-Review+2 (1 comment) http://gerrit.cloudera.org:8080/#/c/5152/2/be/src/exec/kudu-table-sink.cc File be/src/exec/kudu-table-sink.cc: PS2, Line 34: define oops, I guess this should really be "static const int". -- To view, visit http://gerrit.cloudera.org:8080/5152 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I47f17a81e4362ab490019382fedc66c25f07080a Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Matthew JacobsGerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4478: Initial Kudu client mem tracking for sink
Matthew Jacobs has posted comments on this change. Change subject: IMPALA-4478: Initial Kudu client mem tracking for sink .. Patch Set 2: Code-Review+2 carrying +2 -- To view, visit http://gerrit.cloudera.org:8080/5152 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I47f17a81e4362ab490019382fedc66c25f07080a Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Matthew JacobsGerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4478: Initial Kudu client mem tracking for sink
Tim Armstrong has posted comments on this change. Change subject: IMPALA-4478: Initial Kudu client mem tracking for sink .. Patch Set 1: Code-Review+1 (1 comment) http://gerrit.cloudera.org:8080/#/c/5152/1/be/src/exec/kudu-table-sink.cc File be/src/exec/kudu-table-sink.cc: Line 49: " required (bytes) for a KuduTableSink. This flag is subject to change or removal."); Can you add something here that indicates that it is typically 2 * kudu_mutation_buffer_size. Otherwise there's no info in the help text that the two options are correlated. -- To view, visit http://gerrit.cloudera.org:8080/5152 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I47f17a81e4362ab490019382fedc66c25f07080a Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Matthew JacobsGerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4478: Initial Kudu client mem tracking for sink
Dan Hecht has posted comments on this change. Change subject: IMPALA-4478: Initial Kudu client mem tracking for sink .. Patch Set 1: Code-Review+2 (1 comment) http://gerrit.cloudera.org:8080/#/c/5152/1/be/src/exec/kudu-table-sink.cc File be/src/exec/kudu-table-sink.cc: PS1, Line 34: 100 * 1024 * 1024 how about pulling this into a #define and using it here and below, to make it impossible to miss the connection. -- To view, visit http://gerrit.cloudera.org:8080/5152 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I47f17a81e4362ab490019382fedc66c25f07080a Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Matthew JacobsGerrit-Reviewer: Dan Hecht Gerrit-HasComments: Yes