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

crush: fix the problem can not find the define item below the bucket #8141

Merged
merged 1 commit into from May 20, 2016

Conversation

songbaisen
Copy link

@songbaisen songbaisen commented Mar 16, 2016

crush: fix the problem can not find the define item below the bucket

Fixes: #15154
Signed-off-by: song baisen song.baisen@zte.com.cn

@songbaisen
Copy link
Author

When we define the item below the bucket in the map.txt.Then we compile the map,txt

to the map.bin.It will error for cannnot find the item. It is very amazing for usr that

who have define it,but the ceph told him than can not find it.SO slove it!

@liewegas
Copy link
Member

liewegas commented May 3, 2016

Can you add a test case that demonstrates the bug? We aren't follwoing what the problem is. Thanks!

@songbaisen
Copy link
Author

@liewegas Ok,SIr. I will add it later.

@songbaisen songbaisen force-pushed the song14 branch 2 times, most recently from 90a47eb to afbd15b Compare May 13, 2016 09:45
@songbaisen
Copy link
Author

@liewegas :Hello.Sir. I have add the test.Thank you!

@songbaisen
Copy link
Author

songbaisen commented May 13, 2016

@liewegas The unit test error as below looks like the Jenkins bug.
FATAL: org.acegisecurity.providers.UsernamePasswordAuthenticationToken cannot be cast to

And somebody have report this error to Jenkins as below.
https://issues.jenkins-ci.org/browse/JENKINS-34775

return -1; }
else {
std::iter_swap(bucket_itrer[buckets[i]],bucket_itrer[buckets[j]]);
}
Copy link
Member

@liewegas liewegas May 13, 2016

Choose a reason for hiding this comment

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

Please clean up upt eh whitespace:

  • for ( not for(
  • if ( not if(
  • } else { not {\nelse {
  • avoid long lines > 80 chars
  • please include { } on the outer for loop

Thanks!

Copy link
Author

Choose a reason for hiding this comment

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

@liewegas Thank you sir.Done it. I will remember this!:smile:

for (unsigned q=3; q < p->children.size()-1; q++) {
iter_t sub = p->children.begin() + q;
string tag = string_node(sub->children[0]);
if (tag == "item") {
Copy link
Contributor

Choose a reason for hiding this comment

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

could you compare the id().to_long() with _bucket_item also, instead of hardwiring it to "item"?

@tchaikov
Copy link
Contributor

and you need to revise your commit message,

Fixes: http://tracker.ceph.com/issues/15154
Signed-off-by: song baisen <song.baisen@zte.com.cn>

@songbaisen songbaisen force-pushed the song14 branch 3 times, most recently from a50b572 to 2c62f56 Compare May 16, 2016 07:36
@songbaisen
Copy link
Author

@tchaikov Thank you sir! Done it! 👍 👍 👍 🍵 🍻

@@ -816,6 +816,49 @@ void CrushCompiler::dump(iter_t const& i, int ind)
}


//Sometimes when defined the bucket above the item which the bucket
Copy link
Contributor

Choose a reason for hiding this comment

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

sorry, i don't follow you. could you put an example here?

Copy link
Author

Choose a reason for hiding this comment

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

@tchaikov : OK sir.How about the example like below?
before adjust "root default{item localhost }" " host localhost"

after adjust "host localhost" "root default{item localhost }"

Copy link
Contributor

Choose a reason for hiding this comment

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

oic.

so the case is:

rack using_foo { item foo }
host foo { ... }

if an item being used by a bucket is defined after that bucket. CRUSH compiler will create a map by which we can not identify that item when selecting in that bucket.

Copy link
Author

Choose a reason for hiding this comment

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

@tchaikov Yes sir.Thank you!

@liewegas
Copy link
Member

liewegas commented May 19, 2016

-    export CEPH_MON="127.0.0.1:17109"
+    export CEPH_MON="127.0.0.1:17119"

(must be a unique port in the test!)

@songbaisen
Copy link
Author

@liewegas Thank you sir! Learn it. The mon must have unique port here is the reason that when begin unit test we will run a lot of unit script.And may be each script may be set up a mon. Is that right? If so where we can known which port we have used.Should we define a file let people know the max port have used.

@liewegas
Copy link
Member

liewegas commented May 20, 2016 via email

@songbaisen
Copy link
Author

@liewegas Oh Thank you sir! Learn it!

@liewegas liewegas merged commit 4f40a24 into ceph:master May 20, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants