Skip to content

[store] test: fix the RaftError::Shutdown issue#1546

Merged
bohutang merged 6 commits into
databendlabs:masterfrom
drmingdrmer:ff
Aug 22, 2021
Merged

[store] test: fix the RaftError::Shutdown issue#1546
bohutang merged 6 commits into
databendlabs:masterfrom
drmingdrmer:ff

Conversation

@drmingdrmer

@drmingdrmer drmingdrmer commented Aug 20, 2021

Copy link
Copy Markdown
Member

I hereby agree to the terms of the CLA available at: https://datafuse.rs/policies/cla/

Summary

[store] fix: snapshot replication timeout will not cause a RaftError
[test]: refactor: CI will save logs at debug level.
  • Improve CI: run unittest with RUST_BACKTRACE=full and RUST_LOG=debug

  • Upload artifect for debugging when there is a failure

  • Refactor: elaborate error log;
    Record source an backtrace when converting anyhow::Error to ErrorCode

[query] fix dep revision
[store] test: fix the RaftError::Shutdown issue

When running unittest, with heavy cpu load, heartbeat message has chance
to be delayed too much.
When this happens, a follower tries to elect itself and revert the
current leader to a follower state.
This causes the leader to discard every in progress request it has
received. And the client receives an inappropriate error
RaftError::Shuttingdown.

This is actually not a bug.

In such case in practise, a client should re-fetch the latest leader
and retry.

For a unittest, we just extend the timeout to let tests pass happily,
even on a poor CI VM.

[store] refactor: refine tracing
  • Every test case creates its own span as a root.
    This way the logging/tracing belonging to a single test is easy to
    grep.

  • Upgrade to async-raft 0.6.2-alpha.11, with enhanced tracing:
    a span is send along with a message through channels.
    Thus a complete workflow can be tracked.

[store] fix: fix flaky test: generic-kv: get-unexpired

Changelog

  • Improvement
  • Build/Testing/CI

Related Issues

@drmingdrmer drmingdrmer added this to the v0.5 milestone Aug 20, 2021
@databend-bot databend-bot added pr-build this PR changes build/testing/ci steps pr-improvement labels Aug 20, 2021
@databend-bot

Copy link
Copy Markdown
Member

Thanks for the contribution!
I have applied any labels matching special text in your PR Changelog.

Please review the labels and make any necessary changes.

3 similar comments
@databend-bot

Copy link
Copy Markdown
Member

Thanks for the contribution!
I have applied any labels matching special text in your PR Changelog.

Please review the labels and make any necessary changes.

@databend-bot

Copy link
Copy Markdown
Member

Thanks for the contribution!
I have applied any labels matching special text in your PR Changelog.

Please review the labels and make any necessary changes.

@databend-bot

Copy link
Copy Markdown
Member

Thanks for the contribution!
I have applied any labels matching special text in your PR Changelog.

Please review the labels and make any necessary changes.

@codecov-commenter

codecov-commenter commented Aug 21, 2021

Copy link
Copy Markdown

Codecov Report

Merging #1546 (7d2c0e9) into master (8606f17) will increase coverage by 0%.
The diff coverage is 96%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master   #1546    +/-   ##
=======================================
  Coverage      73%     73%            
=======================================
  Files         535     535            
  Lines       32862   32981   +119     
=======================================
+ Hits        24076   24254   +178     
+ Misses       8786    8727    -59     
Impacted Files Coverage Δ
store/src/meta_service/meta_service_impl.rs 75% <66%> (ø)
store/src/meta_service/raftmeta.rs 92% <68%> (+<1%) ⬆️
common/stoppable/src/stoppable_test.rs 95% <100%> (ø)
...c/interpreters/interpreter_database_create_test.rs 87% <100%> (ø)
store/src/api/rpc/flight_service_test.rs 96% <100%> (+<1%) ⬆️
store/src/api/rpc/tls_flight_service_test.rs 93% <100%> (+<1%) ⬆️
store/src/api/rpc_service.rs 93% <100%> (+5%) ⬆️
store/src/executor/action_handler_test.rs 94% <100%> (+<1%) ⬆️
store/src/meta_service/meta_service_impl_test.rs 88% <100%> (+<1%) ⬆️
store/src/meta_service/meta_store_test.rs 99% <100%> (+<1%) ⬆️
... and 21 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8606f17...7d2c0e9. Read the comment docs.

- Every test case creates its own span as a root.
  This way the logging/tracing belonging to a single test is easy to
  grep.

- Upgrade to async-raft 0.6.2-alpha.11, with enhanced tracing:
  a span is send along with a message through channels.
  Thus a complete workflow can be tracked.
When running unittest, with heavy cpu load, heartbeat message has chance
to be delayed too much.
When this happens, a follower tries to elect itself and revert the
current leader to a follower state.
This causes the leader to discard every in progress request it has
received. And the client receives an **inappropriate** error
`RaftError::Shuttingdown`.

This is actually not a bug.

In such case in practise, a client should re-fetch the latest leader
and retry.

For a unittest, we just extend the timeout to let tests pass happily,
even on a poor CI VM.
- Improve CI: run unittest with RUST_BACKTRACE=full and RUST_LOG=debug

- Upload artifect for debugging when there is a failure

- Refactor: elaborate error log;
  Record source an backtrace when converting anyhow::Error to ErrorCode
@drmingdrmer
drmingdrmer marked this pull request as ready for review August 21, 2021 20:50
@drmingdrmer drmingdrmer mentioned this pull request Aug 21, 2021
@databend-bot

Copy link
Copy Markdown
Member

CI Passed
Reviewer Approved
Let's Merge

4 similar comments
@databend-bot

Copy link
Copy Markdown
Member

CI Passed
Reviewer Approved
Let's Merge

@databend-bot

Copy link
Copy Markdown
Member

CI Passed
Reviewer Approved
Let's Merge

@databend-bot

Copy link
Copy Markdown
Member

CI Passed
Reviewer Approved
Let's Merge

@databend-bot

Copy link
Copy Markdown
Member

CI Passed
Reviewer Approved
Let's Merge

@databend-bot

Copy link
Copy Markdown
Member

CI Passed
Reviewer Approved
Let's Merge

2 similar comments
@databend-bot

Copy link
Copy Markdown
Member

CI Passed
Reviewer Approved
Let's Merge

@databend-bot

Copy link
Copy Markdown
Member

CI Passed
Reviewer Approved
Let's Merge

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-build this PR changes build/testing/ci steps

Projects

None yet

Development

Successfully merging this pull request may close these issues.

snapshot_replication test panic [Test] Flaky Unit Test

5 participants