[Impala-ASF-CR] IMPALA-5598: Fix excessive dumping in MemLimitExceeded

2017-08-15 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-5598: Fix excessive dumping in MemLimitExceeded
..


Patch Set 4: Verified+1

-- 
To view, visit http://gerrit.cloudera.org:8080/7597
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib151c3894d4c43b508779e6809d77e2f8af89cf2
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Joe McDonnell 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-5598: Fix excessive dumping in MemLimitExceeded

2017-08-15 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has submitted this change and it was merged.

Change subject: IMPALA-5598: Fix excessive dumping in MemLimitExceeded
..


IMPALA-5598: Fix excessive dumping in MemLimitExceeded

ExecQueryFInstances RPC timeouts in stress tests were
traced to excessive dumping in MemLimitExceeded() and
LogUsage(). The QueryState::Init() hits the process
memory limit, and this causes MemLimitExceeded to dump
the process memory tracker. On the stress test, this
involves dumping hundreds of queries and all the
structures underneath. This is expensive and the
contention between threads accessing these structures
causes RPC timeouts.

This adds the ability to the limit the recursive depth
when dumping memory trackers. It also modifies the
dumping in MemLimitExceeded() to have the following
semantics:
1. The query memory tracker is always logged in full
if it exists.
2. The process memory tracker is logged if the query
memory tracker doesn't exist or if the process memory
limit is being hit. The process memory tracker is
limited to dumping only two levels of children.

Other uses of LogUsage() are not limited (e.g. /memz
and the query memory page on the web UI).

A stress test run with the process memory tracker
limited to two levels showed no RPC timeouts.

Change-Id: Ib151c3894d4c43b508779e6809d77e2f8af89cf2
Reviewed-on: http://gerrit.cloudera.org:8080/7597
Reviewed-by: Dan Hecht 
Tested-by: Impala Public Jenkins
---
M be/src/exec/exec-node.cc
M be/src/exec/hash-table-test.cc
M be/src/runtime/bufferpool/reservation-tracker-test.cc
M be/src/runtime/initial-reservations.cc
M be/src/runtime/mem-tracker.cc
M be/src/runtime/mem-tracker.h
M be/src/runtime/runtime-state.cc
M be/src/service/impala-http-handler.cc
M be/src/util/default-path-handlers.cc
9 files changed, 61 insertions(+), 31 deletions(-)

Approvals:
  Impala Public Jenkins: Verified
  Dan Hecht: Looks good to me, approved



-- 
To view, visit http://gerrit.cloudera.org:8080/7597
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: Ib151c3894d4c43b508779e6809d77e2f8af89cf2
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Joe McDonnell 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] IMPALA-5598: Fix excessive dumping in MemLimitExceeded

2017-08-15 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-5598: Fix excessive dumping in MemLimitExceeded
..


Patch Set 4:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/1063/

-- 
To view, visit http://gerrit.cloudera.org:8080/7597
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib151c3894d4c43b508779e6809d77e2f8af89cf2
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Joe McDonnell 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-5598: Fix excessive dumping in MemLimitExceeded

2017-08-15 Thread Dan Hecht (Code Review)
Dan Hecht has posted comments on this change.

Change subject: IMPALA-5598: Fix excessive dumping in MemLimitExceeded
..


Patch Set 4: Code-Review+2

-- 
To view, visit http://gerrit.cloudera.org:8080/7597
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib151c3894d4c43b508779e6809d77e2f8af89cf2
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Joe McDonnell 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-5598: Fix excessive dumping in MemLimitExceeded

2017-08-15 Thread Joe McDonnell (Code Review)
Hello Tim Armstrong,

I'd like you to reexamine a change.  Please visit

http://gerrit.cloudera.org:8080/7597

to look at the new patch set (#4).

Change subject: IMPALA-5598: Fix excessive dumping in MemLimitExceeded
..

IMPALA-5598: Fix excessive dumping in MemLimitExceeded

ExecQueryFInstances RPC timeouts in stress tests were
traced to excessive dumping in MemLimitExceeded() and
LogUsage(). The QueryState::Init() hits the process
memory limit, and this causes MemLimitExceeded to dump
the process memory tracker. On the stress test, this
involves dumping hundreds of queries and all the
structures underneath. This is expensive and the
contention between threads accessing these structures
causes RPC timeouts.

This adds the ability to the limit the recursive depth
when dumping memory trackers. It also modifies the
dumping in MemLimitExceeded() to have the following
semantics:
1. The query memory tracker is always logged in full
if it exists.
2. The process memory tracker is logged if the query
memory tracker doesn't exist or if the process memory
limit is being hit. The process memory tracker is
limited to dumping only two levels of children.

Other uses of LogUsage() are not limited (e.g. /memz
and the query memory page on the web UI).

A stress test run with the process memory tracker
limited to two levels showed no RPC timeouts.

Change-Id: Ib151c3894d4c43b508779e6809d77e2f8af89cf2
---
M be/src/exec/exec-node.cc
M be/src/exec/hash-table-test.cc
M be/src/runtime/bufferpool/reservation-tracker-test.cc
M be/src/runtime/initial-reservations.cc
M be/src/runtime/mem-tracker.cc
M be/src/runtime/mem-tracker.h
M be/src/runtime/runtime-state.cc
M be/src/service/impala-http-handler.cc
M be/src/util/default-path-handlers.cc
9 files changed, 61 insertions(+), 31 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/97/7597/4
-- 
To view, visit http://gerrit.cloudera.org:8080/7597
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib151c3894d4c43b508779e6809d77e2f8af89cf2
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Joe McDonnell 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] IMPALA-5598: Fix excessive dumping in MemLimitExceeded

2017-08-15 Thread Joe McDonnell (Code Review)
Joe McDonnell has posted comments on this change.

Change subject: IMPALA-5598: Fix excessive dumping in MemLimitExceeded
..


Patch Set 3:

(2 comments)

Also rebased to the latest.

http://gerrit.cloudera.org:8080/#/c/7597/3/be/src/runtime/mem-tracker.cc
File be/src/runtime/mem-tracker.cc:

PS3, Line 353: logged_query_tracker
> rather than introducing another control variable, why not just 'state == nu
Done


http://gerrit.cloudera.org:8080/#/c/7597/3/be/src/runtime/mem-tracker.h
File be/src/runtime/mem-tracker.h:

PS3, Line 338: summary of the queries
> does this mean it it dumps the query-level tracker but nothing below? if so
Yes, it has a single-line summary of the query tracker. I updated the comment 
to indicate this.


-- 
To view, visit http://gerrit.cloudera.org:8080/7597
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib151c3894d4c43b508779e6809d77e2f8af89cf2
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Joe McDonnell 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-5598: Fix excessive dumping in MemLimitExceeded

2017-08-15 Thread Dan Hecht (Code Review)
Dan Hecht has posted comments on this change.

Change subject: IMPALA-5598: Fix excessive dumping in MemLimitExceeded
..


Patch Set 3:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/7597/3/be/src/runtime/mem-tracker.cc
File be/src/runtime/mem-tracker.cc:

PS3, Line 353: logged_query_tracker
rather than introducing another control variable, why not just 'state == 
nullptr'? the comment explains it (especially if you make the comment order 
match the code order of conditionals).


http://gerrit.cloudera.org:8080/#/c/7597/3/be/src/runtime/mem-tracker.h
File be/src/runtime/mem-tracker.h:

PS3, Line 338: summary of the queries
does this mean it it dumps the query-level tracker but nothing below? if so, 
maybe be more explicit about this.


-- 
To view, visit http://gerrit.cloudera.org:8080/7597
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib151c3894d4c43b508779e6809d77e2f8af89cf2
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Joe McDonnell 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-5598: Fix excessive dumping in MemLimitExceeded

2017-08-14 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change.

Change subject: IMPALA-5598: Fix excessive dumping in MemLimitExceeded
..


Patch Set 3: Code-Review+1

-- 
To view, visit http://gerrit.cloudera.org:8080/7597
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib151c3894d4c43b508779e6809d77e2f8af89cf2
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Joe McDonnell 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-5598: Fix excessive dumping in MemLimitExceeded

2017-08-14 Thread Joe McDonnell (Code Review)
Joe McDonnell has posted comments on this change.

Change subject: IMPALA-5598: Fix excessive dumping in MemLimitExceeded
..


Patch Set 2:

(1 comment)

Fix long line and fix a backend test I had missed.

http://gerrit.cloudera.org:8080/#/c/7597/2/be/src/exec/exec-node.cc
File be/src/exec/exec-node.cc:

Line 218:<< 
state->instance_mem_tracker()->LogUsage(MemTracker::UNLIMITED_DEPTH);
> Long line
Done


-- 
To view, visit http://gerrit.cloudera.org:8080/7597
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib151c3894d4c43b508779e6809d77e2f8af89cf2
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Joe McDonnell 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-5598: Fix excessive dumping in MemLimitExceeded

2017-08-14 Thread Joe McDonnell (Code Review)
Hello Tim Armstrong,

I'd like you to reexamine a change.  Please visit

http://gerrit.cloudera.org:8080/7597

to look at the new patch set (#3).

Change subject: IMPALA-5598: Fix excessive dumping in MemLimitExceeded
..

IMPALA-5598: Fix excessive dumping in MemLimitExceeded

ExecQueryFInstances RPC timeouts in stress tests were
traced to excessive dumping in MemLimitExceeded() and
LogUsage(). The QueryState::Init() hits the process
memory limit, and this causes MemLimitExceeded to dump
the process memory tracker. On the stress test, this
involves dumping hundreds of queries and all the
structures underneath. This is expensive and the
contention between threads accessing these structures
causes RPC timeouts.

This adds the ability to the limit the recursive depth
when dumping memory trackers. It also modifies the
dumping in MemLimitExceeded() to have the following
semantics:
1. The query memory tracker is always logged in full
if it exists.
2. The process memory tracker is logged if the query
memory tracker doesn't exist or if the process memory
limit is being hit. The process memory tracker is
limited to dumping only two levels of children.

Other uses of LogUsage() are not limited (e.g. /memz
and the query memory page on the web UI).

A stress test run with the process memory tracker
limited to two levels showed no RPC timeouts.

Change-Id: Ib151c3894d4c43b508779e6809d77e2f8af89cf2
---
M be/src/exec/exec-node.cc
M be/src/exec/hash-table-test.cc
M be/src/runtime/bufferpool/reservation-tracker-test.cc
M be/src/runtime/initial-reservations.cc
M be/src/runtime/mem-tracker.cc
M be/src/runtime/mem-tracker.h
M be/src/runtime/runtime-state.cc
M be/src/service/impala-http-handler.cc
M be/src/util/default-path-handlers.cc
9 files changed, 62 insertions(+), 31 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/97/7597/3
-- 
To view, visit http://gerrit.cloudera.org:8080/7597
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib151c3894d4c43b508779e6809d77e2f8af89cf2
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Joe McDonnell 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] IMPALA-5598: Fix excessive dumping in MemLimitExceeded

2017-08-14 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change.

Change subject: IMPALA-5598: Fix excessive dumping in MemLimitExceeded
..


Patch Set 2: Code-Review+1

(1 comment)

Will let MJ take a look too.

http://gerrit.cloudera.org:8080/#/c/7597/2/be/src/exec/exec-node.cc
File be/src/exec/exec-node.cc:

Line 218:<< 
state->instance_mem_tracker()->LogUsage(MemTracker::UNLIMITED_DEPTH);
Long line


-- 
To view, visit http://gerrit.cloudera.org:8080/7597
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib151c3894d4c43b508779e6809d77e2f8af89cf2
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Joe McDonnell 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-5598: Fix excessive dumping in MemLimitExceeded

2017-08-14 Thread Joe McDonnell (Code Review)
Joe McDonnell has uploaded a new patch set (#2).

Change subject: IMPALA-5598: Fix excessive dumping in MemLimitExceeded
..

IMPALA-5598: Fix excessive dumping in MemLimitExceeded

ExecQueryFInstances RPC timeouts in stress tests were
traced to excessive dumping in MemLimitExceeded() and
LogUsage(). The QueryState::Init() hits the process
memory limit, and this causes MemLimitExceeded to dump
the process memory tracker. On the stress test, this
involves dumping hundreds of queries and all the
structures underneath. This is expensive and the
contention between threads accessing these structures
causes RPC timeouts.

This adds the ability to the limit the recursive depth
when dumping memory trackers. It also modifies the
dumping in MemLimitExceeded() to have the following
semantics:
1. The query memory tracker is always logged in full
if it exists.
2. The process memory tracker is logged if the query
memory tracker doesn't exist or if the process memory
limit is being hit. The process memory tracker is
limited to dumping only two levels of children.

Other uses of LogUsage() are not limited (e.g. /memz
and the query memory page on the web UI).

A stress test run with the process memory tracker
limited to two levels showed no RPC timeouts.

Change-Id: Ib151c3894d4c43b508779e6809d77e2f8af89cf2
---
M be/src/exec/exec-node.cc
M be/src/exec/hash-table-test.cc
M be/src/runtime/initial-reservations.cc
M be/src/runtime/mem-tracker.cc
M be/src/runtime/mem-tracker.h
M be/src/runtime/runtime-state.cc
M be/src/service/impala-http-handler.cc
M be/src/util/default-path-handlers.cc
8 files changed, 61 insertions(+), 30 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/97/7597/2
-- 
To view, visit http://gerrit.cloudera.org:8080/7597
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib151c3894d4c43b508779e6809d77e2f8af89cf2
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Joe McDonnell 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] IMPALA-5598: Fix excessive dumping in MemLimitExceeded

2017-08-14 Thread Joe McDonnell (Code Review)
Joe McDonnell has posted comments on this change.

Change subject: IMPALA-5598: Fix excessive dumping in MemLimitExceeded
..


Patch Set 1:

(3 comments)

Rebased all the way.

http://gerrit.cloudera.org:8080/#/c/7597/1/be/src/runtime/mem-tracker.cc
File be/src/runtime/mem-tracker.cc:

PS1, Line 341: (process_capacity < failed_allocation_size)
> nit: unnecessary parens.
Done


PS1, Line 344: 1
> +1 I was wondering about this as well. The pool tracker is probably not goi
Changed this to two levels. Stress tests show that this isn't a problem. I also 
added constants for the code to use.


http://gerrit.cloudera.org:8080/#/c/7597/1/be/src/runtime/mem-tracker.h
File be/src/runtime/mem-tracker.h:

PS1, Line 324:   int max_recursive_levels = INT_MAX
> Maybe we should make it a required argument to force new callsites to think
Changed this to a required argument.


-- 
To view, visit http://gerrit.cloudera.org:8080/7597
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib151c3894d4c43b508779e6809d77e2f8af89cf2
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Joe McDonnell 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-5598: Fix excessive dumping in MemLimitExceeded

2017-08-06 Thread Matthew Jacobs (Code Review)
Matthew Jacobs has posted comments on this change.

Change subject: IMPALA-5598: Fix excessive dumping in MemLimitExceeded
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7597/1/be/src/runtime/mem-tracker.cc
File be/src/runtime/mem-tracker.cc:

PS1, Line 344: 1
> What do you think about logging 2 levels down to get the query MemTrackers?
+1 I was wondering about this as well. The pool tracker is probably not going 
to be particularly helpful when you hit the proc mem limit and want to know who 
the offender was.


-- 
To view, visit http://gerrit.cloudera.org:8080/7597
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib151c3894d4c43b508779e6809d77e2f8af89cf2
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Joe McDonnell 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-5598: Fix excessive dumping in MemLimitExceeded

2017-08-06 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change.

Change subject: IMPALA-5598: Fix excessive dumping in MemLimitExceeded
..


Patch Set 1:

(3 comments)

Looks good, just a couple of minor things.

http://gerrit.cloudera.org:8080/#/c/7597/1/be/src/runtime/mem-tracker.cc
File be/src/runtime/mem-tracker.cc:

PS1, Line 341: (process_capacity < failed_allocation_size)
nit: unnecessary parens.


PS1, Line 344: 1
What do you think about logging 2 levels down to get the query MemTrackers? 
Might be overkill but otoh the query IDs are useful to track down the offenders 
(at least until we have a better means of tracking down which queries are using 
memory on which backends).

Maybe we should make these values constants as well. E.g. 
POOL_MEMTRACKER_DEPTH, QUERY_MEMTRACKER_DEPTH.


http://gerrit.cloudera.org:8080/#/c/7597/1/be/src/runtime/mem-tracker.h
File be/src/runtime/mem-tracker.h:

PS1, Line 324:   int max_recursive_levels = INT_MAX
Maybe we should make it a required argument to force new callsites to think 
about whether it makes sense to dump the full thing?


-- 
To view, visit http://gerrit.cloudera.org:8080/7597
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib151c3894d4c43b508779e6809d77e2f8af89cf2
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Joe McDonnell 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-5598: Fix excessive dumping in MemLimitExceeded

2017-08-04 Thread Joe McDonnell (Code Review)
Joe McDonnell has uploaded a new change for review.

  http://gerrit.cloudera.org:8080/7597

Change subject: IMPALA-5598: Fix excessive dumping in MemLimitExceeded
..

IMPALA-5598: Fix excessive dumping in MemLimitExceeded

ExecQueryFInstances RPC timeouts in stress tests were
traced to excessive dumping in MemLimitExceeded() and
LogUsage(). The QueryState::Init() hits the process
memory limit, and this causes MemLimitExceeded to dump
the process memory tracker. On the stress test, this
involves dumping hundreds of queries and all the
structures underneath. This is expensive and the
contention between threads accessing these structures
causes RPC timeouts.

This adds the ability to the limit the recursive depth
when dumping memory trackers. It also modifies the
dumping in MemLimitExceeded() to have the following
semantics:
1. The query memory tracker is always logged in full
if it exists.
2. The process memory tracker is logged if the query
memory tracker doesn't exist or if the process memory
limit is being hit. The process memory tracker is
limited to dumping only its immediate children.

Other uses of LogUsage() are not limited (e.g. /memz
and the query memory page on the web UI).

A stress test run with the process memory tracker
limited to dumping its immediate children showed no
RPC timeouts.

Change-Id: Ib151c3894d4c43b508779e6809d77e2f8af89cf2
---
M be/src/runtime/mem-tracker.cc
M be/src/runtime/mem-tracker.h
2 files changed, 39 insertions(+), 18 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/97/7597/1
-- 
To view, visit http://gerrit.cloudera.org:8080/7597
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib151c3894d4c43b508779e6809d77e2f8af89cf2
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Joe McDonnell