You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Henry Robinson (Code Review)" <ge...@cloudera.org> on 2016/09/21 22:50:38 UTC

[Impala-ASF-CR] IMPALA-4011: Remove / reword messages when statestore messages are late

Henry Robinson has uploaded a new change for review.

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

Change subject: IMPALA-4011: Remove / reword messages when statestore messages are late
......................................................................

IMPALA-4011: Remove / reword messages when statestore messages are late

Change-Id: I09c7fa4a94065965e5cb83a3b183b2175f8b45fc
---
M be/src/statestore/statestore.cc
1 file changed, 12 insertions(+), 12 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I09c7fa4a94065965e5cb83a3b183b2175f8b45fc
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson <he...@cloudera.com>

[Impala-ASF-CR] IMPALA-4011: Remove / reword messages when statestore messages are late

Posted by "Henry Robinson (Code Review)" <ge...@cloudera.org>.
Henry Robinson has posted comments on this change.

Change subject: IMPALA-4011: Remove / reword messages when statestore messages are late
......................................................................


Patch Set 2:

Before we consider adding a histogram, we'll need one that can be reset (since we want to focus on the current window of samples). That's in patch https://gerrit.cloudera.org/#/c/4516/.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I09c7fa4a94065965e5cb83a3b183b2175f8b45fc
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Internal Jenkins
Gerrit-Reviewer: Juan Yu <jy...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4011: Remove / reword messages when statestore messages are late

Posted by "Juan Yu (Code Review)" <ge...@cloudera.org>.
Juan Yu has posted comments on this change.

Change subject: IMPALA-4011: Remove / reword messages when statestore messages are late
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4500/2/be/src/statestore/statestore.cc
File be/src/statestore/statestore.cc:

PS2, Line 638: Don't warn for topic updates 
> What do you suggest? The problem I see is that there's no SLA associated wi
Yes, I am thinking similar. Give warning if delay happens continuously for certain period, like 5 or 10 min.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I09c7fa4a94065965e5cb83a3b183b2175f8b45fc
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Internal Jenkins
Gerrit-Reviewer: Juan Yu <jy...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4011: Remove / reword messages when statestore messages are late

Posted by "Matthew Jacobs (Code Review)" <ge...@cloudera.org>.
Matthew Jacobs has posted comments on this change.

Change subject: IMPALA-4011: Remove / reword messages when statestore messages are late
......................................................................


Patch Set 2: Code-Review+1

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4500/1/be/src/statestore/statestore.cc
File be/src/statestore/statestore.cc:

PS1, Line 630: const string& msg = Substitute(
             :           "Missed subscriber ($0) $1 d
> Ok, although bear in mind that CM references aren't verboten just because t
Oh ok, I assumed CM refs would be out. I'm fine either way then.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I09c7fa4a94065965e5cb83a3b183b2175f8b45fc
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4011: Remove / reword messages when statestore messages are late

Posted by "Henry Robinson (Code Review)" <ge...@cloudera.org>.
Henry Robinson has submitted this change and it was merged.

Change subject: IMPALA-4011: Remove / reword messages when statestore messages are late
......................................................................


IMPALA-4011: Remove / reword messages when statestore messages are late

Change-Id: I09c7fa4a94065965e5cb83a3b183b2175f8b45fc
Reviewed-on: http://gerrit.cloudera.org:8080/4500
Reviewed-by: Matthew Jacobs <mj...@cloudera.com>
Tested-by: Internal Jenkins
Reviewed-by: Juan Yu <jy...@cloudera.com>
Reviewed-by: Henry Robinson <he...@cloudera.com>
---
M be/src/statestore/statestore.cc
1 file changed, 12 insertions(+), 14 deletions(-)

Approvals:
  Juan Yu: Looks good to me, but someone else must approve
  Henry Robinson: Looks good to me, approved
  Matthew Jacobs: Looks good to me, but someone else must approve
  Internal Jenkins: Verified



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I09c7fa4a94065965e5cb83a3b183b2175f8b45fc
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Internal Jenkins
Gerrit-Reviewer: Juan Yu <jy...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>

[Impala-ASF-CR] IMPALA-4011: Remove / reword messages when statestore messages are late

Posted by "Internal Jenkins (Code Review)" <ge...@cloudera.org>.
Internal Jenkins has posted comments on this change.

Change subject: IMPALA-4011: Remove / reword messages when statestore messages are late
......................................................................


Patch Set 2: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I09c7fa4a94065965e5cb83a3b183b2175f8b45fc
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Internal Jenkins
Gerrit-Reviewer: Juan Yu <jy...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4011: Remove / reword messages when statestore messages are late

Posted by "Henry Robinson (Code Review)" <ge...@cloudera.org>.
Henry Robinson has posted comments on this change.

Change subject: IMPALA-4011: Remove / reword messages when statestore messages are late
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4500/2/be/src/statestore/statestore.cc
File be/src/statestore/statestore.cc:

PS2, Line 638: Don't warn for topic updates 
> I am a bit concerned about not warn topic delay at all. with the log, as le
What do you suggest? The problem I see is that there's no SLA associated with delivering topic updates, and delivery is pretty variable (initial updates of the full catalog tend to run slower). So it's not clear what threshold to warn on, and even if action should be taken if it's just a period of heavy load. 

I think what might be better is to keep a measure of topic update delays like a histogram so that we can see how topic updates are doing at any time. What do you think?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I09c7fa4a94065965e5cb83a3b183b2175f8b45fc
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Internal Jenkins
Gerrit-Reviewer: Juan Yu <jy...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4011: Remove / reword messages when statestore messages are late

Posted by "Henry Robinson (Code Review)" <ge...@cloudera.org>.
Henry Robinson has posted comments on this change.

Change subject: IMPALA-4011: Remove / reword messages when statestore messages are late
......................................................................


Patch Set 1:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/4500/1/be/src/statestore/statestore.cc
File be/src/statestore/statestore.cc:

PS1, Line 630: // TODO: This should be a healthcheck in a monitored metric in CM, which would
             :       // require a 'rate' metric type.
> remove this reference to CM while we're here
Ok, although bear in mind that CM references aren't verboten just because this is an Apache project. Happy to remove here.


PS1, Line 633: "Missed subscriber ($0) $1 deadline by $2ms, "
             :           "consider increasing --statestore_heartbeat_frequency_ms (currently $3) on "
             :           "this Statestore and --statestore_subscriber_timeout_seconds "
             :           "on subscribers",
> I'm worried it's too hard to know what value to pick. Do we have any guidan
I don't think there's any good advice out there, nor is there in the docs. At some point I think we have to say "you figure it out" with these kinds of warning messages, and it's not helpful necessarily to print a wall of text. A good value is usually a larger one (as recommended), so users have a chance of improving things because the amount by which deadlines were missed will go down so there's a feedback loop.


PS1, Line 636: subscribers
> subscribers (Impala Daemons and the Catalog Server)
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I09c7fa4a94065965e5cb83a3b183b2175f8b45fc
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4011: Remove / reword messages when statestore messages are late

Posted by "Henry Robinson (Code Review)" <ge...@cloudera.org>.
Henry Robinson has uploaded a new patch set (#2).

Change subject: IMPALA-4011: Remove / reword messages when statestore messages are late
......................................................................

IMPALA-4011: Remove / reword messages when statestore messages are late

Change-Id: I09c7fa4a94065965e5cb83a3b183b2175f8b45fc
---
M be/src/statestore/statestore.cc
1 file changed, 12 insertions(+), 14 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I09c7fa4a94065965e5cb83a3b183b2175f8b45fc
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>

[Impala-ASF-CR] IMPALA-4011: Remove / reword messages when statestore messages are late

Posted by "Matthew Jacobs (Code Review)" <ge...@cloudera.org>.
Matthew Jacobs has posted comments on this change.

Change subject: IMPALA-4011: Remove / reword messages when statestore messages are late
......................................................................


Patch Set 1:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/4500/1/be/src/statestore/statestore.cc
File be/src/statestore/statestore.cc:

PS1, Line 630: // TODO: This should be a healthcheck in a monitored metric in CM, which would
             :       // require a 'rate' metric type.
remove this reference to CM while we're here


PS1, Line 636: subscribers
subscribers (Impala Daemons and the Catalog Server)


PS1, Line 633: "Missed subscriber ($0) $1 deadline by $2ms, "
             :           "consider increasing --statestore_heartbeat_frequency_ms (currently $3) on "
             :           "this Statestore and --statestore_subscriber_timeout_seconds "
             :           "on subscribers",
I'm worried it's too hard to know what value to pick. Do we have any guidance we can point to? Or maybe just add "Please consult the Impala documentation for more information."


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I09c7fa4a94065965e5cb83a3b183b2175f8b45fc
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4011: Remove / reword messages when statestore messages are late

Posted by "Juan Yu (Code Review)" <ge...@cloudera.org>.
Juan Yu has posted comments on this change.

Change subject: IMPALA-4011: Remove / reword messages when statestore messages are late
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4500/2/be/src/statestore/statestore.cc
File be/src/statestore/statestore.cc:

PS2, Line 638: Don't warn for topic updates 
I am a bit concerned about not warn topic delay at all. with the log, as least user aware of the topic delay and they can investigate and do some tuning. and it will explain why a new created table by Impala not show quickly on another impalad. On large cluster, I have seen the topic update never able to catch up due to small thread pool.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I09c7fa4a94065965e5cb83a3b183b2175f8b45fc
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Juan Yu <jy...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4011: Remove / reword messages when statestore messages are late

Posted by "Henry Robinson (Code Review)" <ge...@cloudera.org>.
Henry Robinson has posted comments on this change.

Change subject: IMPALA-4011: Remove / reword messages when statestore messages are late
......................................................................


Patch Set 2: Code-Review+2

I'm going to submit this as is, then someone can take on warning when updates are too late over a fixed period.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I09c7fa4a94065965e5cb83a3b183b2175f8b45fc
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Internal Jenkins
Gerrit-Reviewer: Juan Yu <jy...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4011: Remove / reword messages when statestore messages are late

Posted by "Juan Yu (Code Review)" <ge...@cloudera.org>.
Juan Yu has posted comments on this change.

Change subject: IMPALA-4011: Remove / reword messages when statestore messages are late
......................................................................


Patch Set 2: Code-Review+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I09c7fa4a94065965e5cb83a3b183b2175f8b45fc
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Internal Jenkins
Gerrit-Reviewer: Juan Yu <jy...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-HasComments: No