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

Docs: Add ccr to the cat thread pool doc test #31442

Merged
merged 6 commits into from
Jun 21, 2018
Merged

Conversation

dnhatn
Copy link
Member

@dnhatn dnhatn commented Jun 19, 2018

Since #31251, we use the default distribution to test docs. With this
distribution, the cat thread-pool response will include the ccr
thread-pool. This commit adjusts /_cat/thread_pool doc to include ccr.

Relates #31251

Since elastic#31251, we use the default distribution to test docs. With this
distribution, the cat thread-pool response will include the ccr
thread-pool. This commit adjusts /_cat/thread_pool doc to include ccr.

Relates elastic#31251
@dnhatn dnhatn added :Distributed Indexing/CCR Issues around the Cross Cluster State Replication features :Docs labels Jun 19, 2018
@dnhatn dnhatn requested a review from nik9000 June 19, 2018 18:48
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-distributed

@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-docs

@dnhatn
Copy link
Member Author

dnhatn commented Jun 19, 2018

The failed test is the one that I changed.

 REPRODUCE WITH: ./gradlew :docs:integTestRunner -Dtests.seed=5A6C4250CE63D6F6 -Dtests.class=org.elasticsearch.smoketest.DocsClientYamlTestSuiteIT -Dtests.method="test {yaml=reference/cat/thread_pool/line_8}" -Dtests.security.manager=true -Dtests.locale=ru -Dtests.timezone=America/Rainy_River
ERROR   1165s | DocsClientYamlTestSuiteIT.test {yaml=reference/cat/thread_pool/line_8} <<< FAILURES!
   > Throwable #1: java.lang.Exception: Test abandoned because suite timeout was reached.

The regex becomes longer, but I am not sure if it's the cause of the timeout.

@dnhatn
Copy link
Member Author

dnhatn commented Jun 19, 2018

@elasticmachine test this please

Copy link
Member

@martijnvg martijnvg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@dnhatn
Copy link
Member Author

dnhatn commented Jun 20, 2018

I think the test was timeout because of the presence of the two ".+" expressions. A regex of ".+" is slow because it is greedy and matches all input (then backs off char by char). I replaced ".+" with "\S+", we are good now.

@dnhatn
Copy link
Member Author

dnhatn commented Jun 21, 2018

Thanks @bleskes and @martijnvg for reviewing

@dnhatn dnhatn merged commit d467be3 into elastic:ccr Jun 21, 2018
@dnhatn dnhatn deleted the cat-threadpool branch June 21, 2018 00:36
dnhatn added a commit that referenced this pull request Jun 21, 2018
Since #31251, we use the default distribution to test docs. With this
distribution, the cat thread-pool response will include the ccr
thread-pool. This commit adjusts /_cat/thread_pool doc to include ccr.

Relates #31251
dnhatn added a commit to dnhatn/elasticsearch that referenced this pull request Aug 27, 2018
The fix proposed in elastic#31442 fails with the oss distro because the added
3dots does not match anything with the default oss while a 3dots
expression requires matching at least one thread pool.

This change adjusts the thread pool list so that it can match both the
oss (without ccr) and default distro (with ccr).
dnhatn added a commit that referenced this pull request Aug 27, 2018
The fix proposed in #31442 fails with the oss distro because the added
3dots does not match anything with the default oss while a 3dots
expression requires matching at least one thread pool.

This change makes an ellipsis optional so the thread_pool list can match
both the oss distro (without ccr) and default distro (with ccr).

Relates #31442
dnhatn added a commit that referenced this pull request Aug 27, 2018
The fix proposed in #31442 fails with the oss distro because the added
3dots does not match anything with the default oss while a 3dots
expression requires matching at least one thread pool.

This change makes an ellipsis optional so the thread_pool list can match
both the oss distro (without ccr) and default distro (with ccr).

Relates #31442
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Distributed Indexing/CCR Issues around the Cross Cluster State Replication features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants