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

osd/PrimaryLogPG: optimal pick_newest_available #12695

Merged
merged 1 commit into from Jan 9, 2017

Conversation

hjwsm1989
Copy link
Contributor

we can get pg_missing_item from is_missing(), no need to find it again.

Signed-off-by: huangjun hjwsm1989@gmail.com

@hjwsm1989
Copy link
Contributor Author

@tchaikov is there bug in build system? i found some PRs build failed due to same error

@liewegas
Copy link
Member

retest this please

assert(pg_log.get_missing().is_missing(oid));
v = pg_log.get_missing().get_items().find(oid)->second.have;
pg_missing_item pmi;
assert(pg_log.get_missing().is_missing(oid, &pmi));
Copy link
Member

Choose a reason for hiding this comment

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

Please do not rely on a side effect of an assert() call. assign the result to a bool local and assert that is true.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

like this ?

  • bool is_missing = pg_log.get_missing().is_missing(oid, &pmi);
  • assert(is_missing);
  • v = pmi.have;

Copy link
Member

Choose a reason for hiding this comment

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

yup

  we can get pg_missing_item from is_missing(), no need to find it again.

  Signed-off-by: huangjun <hjwsm1989@gmail.com>
@liewegas
Copy link
Member

liewegas commented Dec 29, 2016 via email

v = pg_log.get_missing().get_items().find(oid)->second.have;
pg_missing_item pmi;
bool is_missing = pg_log.get_missing().is_missing(oid, &pmi);
assert(is_missing);
Copy link
Contributor

Choose a reason for hiding this comment

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

i think we are moving from assert() to ceph_assert(). see the thread of https://www.spinics.net/lists/ceph-devel/msg31049.html

Copy link
Member

Choose a reason for hiding this comment

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

hasn't happened yet, tho, can be fixed up later

@liewegas
Copy link
Member

retest this please

@liewegas liewegas changed the title osd/PrimaryLogPG: optimal pick_newest_available osd/PrimaryLogPG: optimal pick_newest_available Jan 9, 2017
@liewegas liewegas merged commit 4ca7757 into ceph:master Jan 9, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants