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

fix insert inode fail when load deleting partition #997

Merged

Conversation

cw123
Copy link
Contributor

@cw123 cw123 commented Jan 14, 2022

What problem does this PR solve?

Issue Number: #995

Problem Summary: fix insert inode fail when load deleting partition

What is changed and how it works?

What's Changed:

How it Works:

Side effects(Breaking backward compatibility? Performance regression?):

Check List

  • Relevant documentation/comments is changed or added
  • I acknowledge that all my contributions will be made under the project's license

@cw123 cw123 force-pushed the fix_deleting_partition_insert_inode_fail branch 2 times, most recently from 04a3483 to e725958 Compare January 17, 2022 03:40
if (rc != MetaStatusCode::OK) {
LOG(ERROR) << "InsertInode failed, retCode = " << rc;
LOG(ERROR) << "InsertInode failed, retCode = "
Copy link
Member

Choose a reason for hiding this comment

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

using “:” instead of “=” in log

Copy link
Contributor

@wu-hanqing wu-hanqing left a comment

Choose a reason for hiding this comment

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

you should add tests to verify this patch.

@@ -137,6 +130,20 @@ bool MetaStoreImpl::Load(const std::string& pathname) {
partitionMap_.clear();
LOG(ERROR) << "Load metadata failed.";
}

for (auto it = partitionMap_.begin(); it != partitionMap_.end(); it++) {
Copy link
Contributor

Choose a reason for hiding this comment

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

use range-based for loop

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The two methods are equivalent

@cw123 cw123 force-pushed the fix_deleting_partition_insert_inode_fail branch from e725958 to 63f8fd3 Compare January 17, 2022 07:28
@cw123
Copy link
Contributor Author

cw123 commented Jan 17, 2022

you should add tests to verify this patch.

done, add test case

@@ -94,7 +94,10 @@ class MetastoreTest : public ::testing::Test {
first.partitionid() == second.partitionid() &&
first.start() == second.start() &&
first.end() == second.end() &&
first.nextid() == second.nextid();
first.nextid() == second.nextid() &&
Copy link
Contributor

Choose a reason for hiding this comment

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

you can use google::protobuf::util::MessageDifferencer::Equals to simplify this compare

@cw123
Copy link
Contributor Author

cw123 commented Jan 17, 2022

recheck

2 similar comments
@cw123
Copy link
Contributor Author

cw123 commented Jan 18, 2022

recheck

@cw123
Copy link
Contributor Author

cw123 commented Jan 18, 2022

recheck

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants