Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/9282 )
Change subject: IMPALA-6116: Bound memory usage of DataStreamSevice's service queue ...................................................................... Patch Set 4: (1 comment) http://gerrit.cloudera.org:8080/#/c/9282/4/be/src/runtime/mem-tracker.h File be/src/runtime/mem-tracker.h: http://gerrit.cloudera.org:8080/#/c/9282/4/be/src/runtime/mem-tracker.h@252 PS4, Line 252: skip_gc = false > I think it's going to be kind of hard for readers of the code and new calls Yes, we can try to document it. I guess the problem to me is that this function is unconditionally GC'ing when any MemTracker's limit is exceeded. However, IIUC, GC'ing should help if you exceeded the process mem limit. So, it's unclear to me if it's okay to add a check here to see if the mem_tracker exceeding the limit is the process_mem_tracker_ and only call GcMemory in that case. -- To view, visit http://gerrit.cloudera.org:8080/9282 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Idea4262dfb0e0aa8d58ff6ea6a8aaaa248e880b9 Gerrit-Change-Number: 9282 Gerrit-PatchSet: 4 Gerrit-Owner: Michael Ho <[email protected]> Gerrit-Reviewer: Dan Hecht <[email protected]> Gerrit-Reviewer: Lars Volker <[email protected]> Gerrit-Reviewer: Michael Ho <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Comment-Date: Fri, 16 Feb 2018 19:17:14 +0000 Gerrit-HasComments: Yes
