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

parse diskPartsKey and add unit test #3416

Merged
merged 7 commits into from
Dec 8, 2021

Conversation

Nivras
Copy link
Contributor

@Nivras Nivras commented Dec 6, 2021

What type of PR is this?

  • bug
  • feature
  • enhancement

What does this PR do?

add functions to parse host, spaceId and path in diskPartsKey, and fix parseDiskPartsSpace().
add unit test for MetaUtilKeys and DiskManager

Which issue(s)/PR(s) this PR relates to?

#3369

Special notes for your reviewer, ex. impact of this fix, etc:

Additional context:

Checklist:

  • Documentation affected (Please add the label if documentation needs to be modified.)
  • Incompatible (If it is incompatible, please describe it and add corresponding label.)
  • Need to cherry-pick (If need to cherry-pick to some branches, please label the destination version(s).)
  • Performance impacted: Consumes more CPU/Memory

Release notes:

Please confirm whether to reflect in release notes and how to describe:

                                                            `

@Nivras Nivras added the ready-for-testing PR: ready for the CI test label Dec 6, 2021
critical27
critical27 previously approved these changes Dec 6, 2021
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.

Well done

wenhaocs
wenhaocs previously approved these changes Dec 6, 2021
src/common/utils/MetaKeyUtils.cpp Show resolved Hide resolved
wenhaocs
wenhaocs previously approved these changes Dec 7, 2021
@codecov-commenter
Copy link

codecov-commenter commented Dec 7, 2021

Codecov Report

Merging #3416 (1421f30) into master (d6f83f3) will increase coverage by 0.02%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3416      +/-   ##
==========================================
+ Coverage   85.24%   85.26%   +0.02%     
==========================================
  Files        1278     1278              
  Lines      119200   119254      +54     
==========================================
+ Hits       101608   101684      +76     
+ Misses      17592    17570      -22     
Impacted Files Coverage Δ
src/common/memory/MemoryUtils.cpp 93.33% <100.00%> (+0.65%) ⬆️
src/common/utils/MetaKeyUtils.cpp 82.15% <100.00%> (+0.78%) ⬆️
src/common/utils/test/MetaKeyUtilsTest.cpp 94.63% <100.00%> (+0.30%) ⬆️
src/kvstore/test/DiskManagerTest.cpp 99.19% <100.00%> (+0.21%) ⬆️
src/meta/processors/BaseProcessor.h 75.67% <0.00%> (-16.22%) ⬇️
...c/meta/processors/parts/GetPartsAllocProcessor.cpp 83.05% <0.00%> (-8.48%) ⬇️
src/common/thrift/ThriftClientManager-inl.h 82.97% <0.00%> (-6.39%) ⬇️
src/graph/planner/Planner.cpp 75.00% <0.00%> (-5.00%) ⬇️
src/graph/service/PermissionCheck.cpp 78.72% <0.00%> (-2.13%) ⬇️
src/codec/RowReaderWrapper.h 94.23% <0.00%> (-1.93%) ⬇️
... and 23 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 e33924f...1421f30. Read the comment docs.

Copy link
Contributor

@pengweisong pengweisong 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.

@critical27 critical27 merged commit d38fc07 into vesoft-inc:master Dec 8, 2021
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.

6 participants