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

store admintasks' status and send the status to metad when starting #2843

Merged
merged 3 commits into from
Sep 26, 2021

Conversation

liwenhui-soul
Copy link
Contributor

@liwenhui-soul liwenhui-soul commented Sep 10, 2021

@CLAassistant
Copy link

CLAassistant commented Sep 10, 2021

CLA assistant check
All committers have signed the CLA.

@jievince
Copy link
Contributor

Plz sign the CLA

@liwenhui-soul liwenhui-soul changed the title Lwh store admintasks' status and send the status to metad when starting Sep 10, 2021
@liwenhui-soul liwenhui-soul added the ready-for-testing PR: ready for the CI test label Sep 10, 2021
@liwenhui-soul liwenhui-soul requested review from liuyu85cn and removed request for liuyu85cn September 13, 2021 01:53
@liwenhui-soul liwenhui-soul force-pushed the lwh branch 4 times, most recently from acb921f to f62eb9d Compare September 13, 2021 09:00
@bright-starry-sky
Copy link
Contributor

What is the purpose of this PR? Could you explain the context of this PR?

@liwenhui-soul
Copy link
Contributor Author

What is the purpose of this PR? Could you explain the context of this PR?

the purpose is to solve this problem.#2764

@liwenhui-soul liwenhui-soul force-pushed the lwh branch 2 times, most recently from 13e5ac6 to cc8aba6 Compare September 24, 2021 07:36
@bright-starry-sky
Copy link
Contributor

Please resolve the conflict.

Copy link
Contributor

@critical27 critical27 left a comment

Choose a reason for hiding this comment

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

LGTM now, the flush could be removed

@liwenhui-soul liwenhui-soul force-pushed the lwh branch 2 times, most recently from 40614ed to 8536af3 Compare September 26, 2021 03:41
@liwenhui-soul
Copy link
Contributor Author

Please resolve the conflict.

done

@codecov-commenter
Copy link

codecov-commenter commented Sep 26, 2021

Codecov Report

Merging #2843 (41dae2d) into master (dddf54e) will decrease coverage by 0.71%.
The diff coverage is 28.64%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2843      +/-   ##
==========================================
- Coverage   85.47%   84.76%   -0.72%     
==========================================
  Files        1229     1229              
  Lines      110375   110551     +176     
==========================================
- Hits        94341    93704     -637     
- Misses      16034    16847     +813     
Impacted Files Coverage Δ
src/common/utils/NebulaKeyUtils.cpp 89.03% <0.00%> (-4.89%) ⬇️
src/common/utils/NebulaKeyUtils.h 78.02% <ø> (ø)
src/kvstore/KVEngine.h 100.00% <ø> (ø)
src/kvstore/RocksEngine.cpp 81.79% <0.00%> (-1.73%) ⬇️
src/kvstore/RocksEngine.h 66.66% <0.00%> (-25.00%) ⬇️
src/storage/CommonUtils.h 94.87% <ø> (ø)
src/storage/StorageServer.cpp 0.00% <0.00%> (-81.13%) ⬇️
src/storage/admin/AdminTask.h 87.50% <ø> (ø)
src/storage/admin/AdminTaskProcessor.cpp 0.00% <0.00%> (-69.57%) ⬇️
src/storage/admin/AdminTaskManager.cpp 50.73% <7.14%> (-29.44%) ⬇️
... and 87 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 8b28d77...41dae2d. Read the comment docs.

Copy link
Contributor

@critical27 critical27 left a comment

Choose a reason for hiding this comment

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

Good job, LGTM

Copy link
Contributor

@bright-starry-sky bright-starry-sky left a comment

Choose a reason for hiding this comment

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

Good job . LGTM.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-for-testing PR: ready for the CI test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants