-
Notifications
You must be signed in to change notification settings - Fork 62
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Print the warning if amount of events in incoming queue is too big #75
Print the warning if amount of events in incoming queue is too big #75
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FYI there already is an admin monitor showing this warning in the gerrit trigger.
if (queueSize >= WORK_QUEUE_SIZE_WARNING_THRESHOLD) { | ||
logger.warn("The Gerrit-trigger incoming events queue contains {} items!" | ||
+ " Something might be stuck, or your system can't process the commands fast enough." | ||
+ " Try to increase the number of receiving worker threads on the Gerrit configuration page." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Strange to be talking about configuration pages here. Other consumers are not necessarily a trigger and might not even have a configuration page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To be honest, I just copy-pasted it from
gerrit-events/src/main/java/com/sonymobile/tools/gerrit/gerritevents/GerritSendCommandQueue.java
Line 174 in 7f3547c
private void checkQueueSize() { |
Can fix in both (initially I tried to generalise this two classes, but change grew too fast and did not stop).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah my memory bad :) the old warning was only on the out queue I guess, not the in queue .
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rsandell should I remove mentioning of "configuration pages" from both?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, we can rephrase it a bit I think. Something like:
The Gerrit incoming events queue contains {} items!
Something might be stuck, or your system can't process the commands fast enough.
Try to increase the number of receiving worker threads.
/** | ||
* The minimum size of the job-queue before monitors should begin to warn the administrator(s). | ||
*/ | ||
public static final int WORK_QUEUE_SIZE_WARNING_THRESHOLD = 40; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be nice if this could be set via for example a system property (negative value could be for turning it off), the default is probably OK, there is another default in the gerrit trigger shown via an admin monitor iirc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will do a property. The default in https://github.com/sonyxperiadev/gerrit-events/blob/7f3547c6d55946e25e99a847b5160d69e59994ba/src/main/java/com/sonymobile/tools/gerrit/gerritevents/GerritSendCommandQueue.java is 20, but in our case it was too low.
It should simplify troubleshooting in case of slowness in event processing logic.
fd6815e
to
9e1a872
Compare
Done. @rsandell, please, take a look one more time. |
It should simplify troubleshooting in case of slowness in event
processing logic.