Huaisi Xu has posted comments on this change. Change subject: IMPALA-3613: Do not update unregistered subscribers ......................................................................
Patch Set 2: (2 comments) > (2 comments) > > Did you check the size of the statestore's subscriber queue? > > After 1 hour the subscribers should have sent ~60 * 60 * 3 > registrations, which is 10800, but even with one thread would have > sent ~3600 heartbeats at one every six seconds. So the registration > queue could still be filling up. > > I think it would be useful to have the heartbeat frequency be 30 > minutes in your test. Then after the heartbeats are sent the > statestore should have almost completely cleaned out the queue, and > you can check that the queue size is small. > > That means adding some way to see the size of the queue. You could > just print it periodically in your test environment for starters. I did print that and heartbeat in my test. queue size remained close to 0 all the time instead of gradually increasing over time.(I forgot why though..: )) heartbeat never repeat unexpectedly after the change. http://gerrit.cloudera.org:8080/#/c/3320/2/be/src/statestore/statestore.h File be/src/statestore/statestore.h: PS2, Line 362: the second entry is the subscriber to send it to. > comment needs updating I think it is correct to say that "the second entry is the subscriber to send it to"... if we think the subscriber more unique. Line 363: typedef std::pair<int64_t, std::pair<SubscriberId, TUniqueId>> ScheduledSubscriberUpdate; > nit: long line so close(1 character). do we really have to follow 90 characters in this case? -- To view, visit http://gerrit.cloudera.org:8080/3320 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic573d257b8171bfcab33a1f72de1399be7db10fd Gerrit-PatchSet: 2 Gerrit-Project: Impala Gerrit-Branch: cdh5-trunk Gerrit-Owner: Huaisi Xu <[email protected]> Gerrit-Reviewer: Henry Robinson <[email protected]> Gerrit-Reviewer: Huaisi Xu <[email protected]> Gerrit-HasComments: Yes
