Skip to content
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

bz466228: BlockedThreadChecker doesn't have a config value for the time until a stack trace is written #1028

Merged
merged 5 commits into from May 27, 2015

Conversation

@alexlehm
Copy link
Contributor

alexlehm commented May 3, 2015

https://bugs.eclipse.org/bugs/show_bug.cgi?id=466228

added config value warningExceptionTime to VertxOptions to set the threshold value when a stack trace
is included in the blocking warning. changed the log code to put the stack trace into the log statement
so that it is output by the logger and not to stdout
added a unit test for the BlockChecker class

alexlehm added 2 commits May 3, 2015
…ime until a stack trace is written

added config value warningExceptionTime to VertxOptions to set the threshold value when a stack trace
is included in the blocking warning. changed the log code to put the stack trace into the log statement
so that it is output by the logger and not to stdout
added a unit test for the BlockChecker class

Signed-off-by: alexlehm <alexlehm@gmail.com>
Signed-off-by: alexlehm <alexlehm@gmail.com>
@@ -45,12 +46,14 @@ public void run() {
long execStart = thread.startTime();
long dur = now - execStart;
if (execStart != 0 && dur > (thread.isWorker() ? maxWorkerExecTime : maxEventLoopExecTime)) {

This comment has been minimized.

Copy link
@purplefox

purplefox May 26, 2015

Contributor

duplicate evaulation, better to do it once.

This comment has been minimized.

Copy link
@alexlehm

alexlehm May 26, 2015

Author Contributor

ok, changed it

Signed-off-by: alexlehm <alexlehm@gmail.com>
* @return a reference to this, so the API can be used fluently
*/
public VertxOptions setWarningExceptionTime(long warningExceptionTime) {
this.warningExceptionTime = warningExceptionTime;

This comment has been minimized.

Copy link
@purplefox

purplefox May 26, 2015

Contributor

Need to check argument is not < 1 as in other getters

This comment has been minimized.

Copy link
@alexlehm

alexlehm May 26, 2015

Author Contributor

added

import org.junit.Test;

/**
* @author <a href="http://tfox.org">Tim Fox</a>

This comment has been minimized.

Copy link
@purplefox

purplefox May 26, 2015

Contributor

Did I write this test? ;)

This comment has been minimized.

Copy link
@alexlehm

alexlehm May 26, 2015

Author Contributor

sorry, I copied another test class and exchanged the test methods so the comments ended up there, should I change that?

This comment has been minimized.

Copy link
@alexlehm

alexlehm May 26, 2015

Author Contributor

ok, i changed the author comment

/**
* @author <a href="http://tfox.org">Tim Fox</a>
*/
public class BlockedThreadCheckerTest extends VertxTestBase {

This comment has been minimized.

Copy link
@purplefox

purplefox May 26, 2015

Contributor

I don't really see the point of this test.

What we should do is have tests in VertxOptionsTest as we do with the other settings

This comment has been minimized.

Copy link
@alexlehm

alexlehm May 26, 2015

Author Contributor

main reason for the test was to get 100% coverage, actually asserting the logging calls is a bit difficult since it requires a log mock of some kind

This comment has been minimized.

Copy link
@alexlehm

alexlehm May 26, 2015

Author Contributor

i added tests to VertxOptionsTest

alexlehm added 2 commits May 26, 2015
added tests for the warningExceptionTime property

Signed-off-by: alexlehm <alexlehm@gmail.com>
Signed-off-by: alexlehm <alexlehm@gmail.com>
@alexlehm

This comment has been minimized.

Copy link
Contributor Author

alexlehm commented May 26, 2015

ok, i think i implemented the necessary chanced, can you please do a new review

@purplefox

This comment has been minimized.

Copy link
Contributor

purplefox commented May 27, 2015

thanks alex

purplefox added a commit that referenced this pull request May 27, 2015
bz466228:  BlockedThreadChecker doesn't have a config value for the time until a stack trace is written
@purplefox purplefox merged commit ea4bd5c into eclipse-vertx:master May 27, 2015
1 check passed
1 check passed
ip-validation
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.