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

Add timeout for starting tarantool server #302

Merged
merged 4 commits into from
Mar 15, 2022

Conversation

VitaliyaIoffe
Copy link
Contributor

@VitaliyaIoffe VitaliyaIoffe commented May 17, 2021

This patch includes 4 commits:

Use gevent sleep for switching context

time.sleep was changed to gevent.sleep to allow current greenlet to
sleep and others to run. When time.sleep is used, greenlet's context
is not changed from the main process to test's greenlet.

As a result of this, there is no data received by the main process while
hanging the tarantool server process and the test is fallen down by the
common timeout (NO_OUTPUT_TIMEOUT). Even worse, the tarantool server
process is not killed by test-run.

Using gevent.sleep makes the test fail by the test timeout and kill
the farantool server process.

Part of #276

Add timeout for starting tarantool server

When a tarantool server starts, it waits for a special pattern in the
log file to proceed. If there is no pattern present, the server hangs.
After the test timeout (TEST_TIMEOUT) runs out, the test fails.

This patch adds the --server-start-timeout option to test-run (by
default it equals to 90 seconds). Now when the server hangs and the
time (SERVER_START_TIMEOUT) runs out, a comprehensible exception is
raised with the message that the server failed to start within the
timeout.

Fixes: #276

Kill processes of non-started tarantool servers

It was found that processes of non-started tarantool servers are not
killed by test-run and leave to hang. This situation can be reproduced
by creating the main server, then creating a replica server, but the
replica server is unable to join the master, for example, due to lack
of user permissions. In this case, the test fails by the server start
timeout and test-run kills the main server process only.

Fixes #256
Follows up #276

Add unit test checking no hanging procs present

This patch adds a simple unit test checking that if a tarantool server
failed to start within a certain amount of seconds, test-tun raises a
comprehensible exception and kills the server process.

Follows up #256
Follows up #276

@kyukhin kyukhin requested a review from Totktonada May 17, 2021 11:07
@VitaliyaIoffe VitaliyaIoffe force-pushed the VitaliyaIoffe/gh-276-hangs-timeout-doesnt-work branch 2 times, most recently from 5864c82 to 7135e26 Compare May 17, 2021 15:05
lib/tarantool_server.py Outdated Show resolved Hide resolved
lib/tarantool_server.py Outdated Show resolved Hide resolved
lib/preprocessor.py Outdated Show resolved Hide resolved
lib/tarantool_server.py Outdated Show resolved Hide resolved
@VitaliyaIoffe VitaliyaIoffe force-pushed the VitaliyaIoffe/gh-276-hangs-timeout-doesnt-work branch 4 times, most recently from 1d40f4e to 25bbb47 Compare May 21, 2021 11:48
@VitaliyaIoffe VitaliyaIoffe requested a review from Totktonada May 22, 2021 09:04
lib/tarantool_server.py Outdated Show resolved Hide resolved
lib/preprocessor.py Outdated Show resolved Hide resolved
@ligurio
Copy link
Member

ligurio commented May 27, 2021

@VitaliyaIoffe could you rebase to master? Latest changes includes code coverage measurement and it would be interesting how code coverage numbers are changed with your tests.

@VitaliyaIoffe VitaliyaIoffe force-pushed the VitaliyaIoffe/gh-276-hangs-timeout-doesnt-work branch 8 times, most recently from f612ae4 to 241b0d8 Compare May 31, 2021 16:54
lib/preprocessor.py Outdated Show resolved Hide resolved
@ylobankov ylobankov force-pushed the VitaliyaIoffe/gh-276-hangs-timeout-doesnt-work branch 2 times, most recently from 68d3d41 to 2a374c7 Compare March 10, 2022 13:29
VitaliyaIoffe and others added 2 commits March 10, 2022 17:31
When a tarantool server starts, it waits for a special pattern in the
log file to proceed. If there is no pattern present, the server hangs.
After the test timeout (TEST_TIMEOUT) runs out, the test fails.

This patch adds the `--server-start-timeout` option to test-run (by
default it equals to 90 seconds). Now when the server hangs and the
time (SERVER_START_TIMEOUT) runs out, a comprehensible exception is
raised with the message that the server failed to start within the
timeout.

Fixes: #276
It was found that processes of non-started tarantool servers are not
killed by test-run and leave to hang. This situation can be reproduced
by creating the main server, then creating a replica server, but the
replica server is unable to join the master, for example, due to lack
of user permissions. In this case, the test fails by the server start
timeout and test-run kills the main server process only. This patch
fixes the issue.

Fixes #256
Follows up #276
@ylobankov ylobankov force-pushed the VitaliyaIoffe/gh-276-hangs-timeout-doesnt-work branch from 2a374c7 to 03d260a Compare March 10, 2022 13:56
@ylobankov ylobankov requested review from ligurio and Totktonada March 10, 2022 14:00
@ylobankov
Copy link
Contributor

Thanks for fixes!

See my comments inline and one more note: please put commit "Add unit test checking no hanging procs present" right after commit "Add timeout for starting tarantool server" with fix or squash them to a single commit.

It is not possible. The test checks that a process of a non-started tarantool server will not hang and will be killed after server start timeout runs out. The problem with hanging processes of non-started tarantool servers was resolved in the Kill processes of non-started tarantool servers commit and the test can be added only after this commit.

lib/options.py Show resolved Hide resolved
lib/tarantool_server.py Show resolved Hide resolved
test-run.py Show resolved Hide resolved
This patch adds a simple unit test checking that if a tarantool server
failed to start within a certain amount of seconds, test-tun raises a
comprehensible exception and kills the server process.

Follows up #256
Follows up #276
@ylobankov ylobankov force-pushed the VitaliyaIoffe/gh-276-hangs-timeout-doesnt-work branch from 03d260a to 37bd48f Compare March 11, 2022 12:32
@ylobankov ylobankov requested a review from ligurio March 11, 2022 12:35
Copy link
Member

@Totktonada Totktonada left a comment

Choose a reason for hiding this comment

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

LGTM.

Copy link
Member

@ligurio ligurio left a comment

Choose a reason for hiding this comment

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

LGTM

@ylobankov ylobankov merged commit 3bb97ec into master Mar 15, 2022
@ylobankov ylobankov deleted the VitaliyaIoffe/gh-276-hangs-timeout-doesnt-work branch March 15, 2022 09:02
kyukhin added a commit to kyukhin/tarantool that referenced this pull request Mar 18, 2022
Bump test-run to new version with the following improvements:
- Add timeout for starting tarantool server (tarantool/test-run#302)
- Kill hanging processes of not started servers (tarantool/test-run#332)
- Rerun all failed tests, not only marked as fragile (tarantool/test-run#329)

NO_DOC=testing
NO_TEST=testing
NO_CHANGELOG=testing
kyukhin added a commit to tarantool/tarantool that referenced this pull request Mar 18, 2022
Bump test-run to new version with the following improvements:
- Add timeout for starting tarantool server (tarantool/test-run#302)
- Kill hanging processes of not started servers (tarantool/test-run#332)
- Rerun all failed tests, not only marked as fragile (tarantool/test-run#329)

NO_DOC=testing
NO_TEST=testing
NO_CHANGELOG=testing
kyukhin added a commit to tarantool/tarantool that referenced this pull request Mar 18, 2022
Bump test-run to new version with the following improvements:
- Add timeout for starting tarantool server (tarantool/test-run#302)
- Kill hanging processes of not started servers (tarantool/test-run#332)
- Rerun all failed tests, not only marked as fragile (tarantool/test-run#329)

NO_DOC=testing
NO_TEST=testing
NO_CHANGELOG=testing
kyukhin added a commit to tarantool/tarantool that referenced this pull request Mar 18, 2022
Bump test-run to new version with the following improvements:
- Add timeout for starting tarantool server (tarantool/test-run#302)
- Kill hanging processes of not started servers (tarantool/test-run#332)
- Rerun all failed tests, not only marked as fragile
  (tarantool/test-run#329)

NO_DOC=testing
NO_TEST=testing
NO_CHANGELOG=testing
kyukhin added a commit to tarantool/tarantool that referenced this pull request Mar 18, 2022
Bump test-run to new version with the following improvements:
- Add timeout for starting tarantool server (tarantool/test-run#302)
- Kill hanging processes of not started servers (tarantool/test-run#332)
- Rerun all failed tests, not only marked as fragile
  (tarantool/test-run#329)

NO_DOC=testing
NO_TEST=testing
NO_CHANGELOG=testing
kyukhin added a commit to tarantool/tarantool that referenced this pull request Mar 18, 2022
Bump test-run to new version with the following improvements:
- Add timeout for starting tarantool server (tarantool/test-run#302)
- Kill hanging processes of not started servers (tarantool/test-run#332)
- Rerun all failed tests, not only marked as fragile
  (tarantool/test-run#329)

NO_DOC=testing
NO_TEST=testing
NO_CHANGELOG=testing
kyukhin added a commit to tarantool/tarantool that referenced this pull request Mar 21, 2022
Bump test-run to new version with the following improvements:
- Add timeout for starting tarantool server (tarantool/test-run#302)
- Kill hanging processes of not started servers (tarantool/test-run#332)
- Rerun all failed tests, not only marked as fragile
  (tarantool/test-run#329)

NO_DOC=testing
NO_TEST=testing
NO_CHANGELOG=testing
kyukhin added a commit to tarantool/tarantool that referenced this pull request Mar 21, 2022
Bump test-run to new version with the following improvements:
- Add timeout for starting tarantool server (tarantool/test-run#302)
- Kill hanging processes of not started servers (tarantool/test-run#332)
- Rerun all failed tests, not only marked as fragile
  (tarantool/test-run#329)

NO_DOC=testing
NO_TEST=testing
NO_CHANGELOG=testing
kyukhin added a commit to tarantool/tarantool that referenced this pull request Mar 21, 2022
Bump test-run to new version with the following improvements:
- Add timeout for starting tarantool server (tarantool/test-run#302)
- Kill hanging processes of not started servers (tarantool/test-run#332)
- Rerun all failed tests, not only marked as fragile
  (tarantool/test-run#329)

NO_DOC=testing
NO_TEST=testing
NO_CHANGELOG=testing
kyukhin added a commit to tarantool/tarantool that referenced this pull request Mar 21, 2022
Bump test-run to new version with the following improvements:
- Add timeout for starting tarantool server (tarantool/test-run#302)
- Kill hanging processes of not started servers (tarantool/test-run#332)
- Rerun all failed tests, not only marked as fragile
  (tarantool/test-run#329)

NO_DOC=testing
NO_TEST=testing
NO_CHANGELOG=testing

(cherry picked from commit 0b46154)
kyukhin added a commit to tarantool/tarantool that referenced this pull request Mar 21, 2022
Bump test-run to new version with the following improvements:
- Add timeout for starting tarantool server (tarantool/test-run#302)
- Kill hanging processes of not started servers (tarantool/test-run#332)
- Rerun all failed tests, not only marked as fragile
  (tarantool/test-run#329)

NO_DOC=testing
NO_TEST=testing
NO_CHANGELOG=testing

(cherry picked from commit 0b46154)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants