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

Omap small bugs #6046

Closed
wants to merge 10 commits into from
Closed

Omap small bugs #6046

wants to merge 10 commits into from

Conversation

majianpeng
Copy link
Member

No description provided.

Signed-off-by: Jianpeng Ma <jianpeng.ma@intel.com>
When set omapheader/omapvals, it alreade set FLAG_OMAP. So for remove
don't set again.

Signed-off-by: Jianpeng Ma <jianpeng.ma@intel.com>
Signed-off-by: Jianpeng Ma <jianpeng.ma@intel.com>
The code already check object whether exist. So it don't need call
touch.

Signed-off-by: Jianpeng Ma <jianpeng.ma@intel.com>
Signed-off-by: Jianpeng Ma <jianpeng.ma@intel.com>
…deep-scrub.

Signed-off-by: Jianpeng Ma <jianpeng.ma@intel.com>
For be_deep_scrub, we already got the hash info attr. So if don't found
in cache, directly decode rather than call stat & getattr.

Signed-off-by: Jianpeng Ma <jianpeng.ma@intel.com>
Now erasure pool don't support omap. So it dont need set those.

Signed-off-by: Jianpeng Ma <jianpeng.ma@intel.com>
Signed-off-by: Jianpeng Ma <jianpeng.ma@intel.com>
… omap.

Signed-off-by: Jianpeng Ma <jianpeng.ma@intel.com>
@majianpeng
Copy link
Member Author

@athanatos . Have you time to review those? thanks

@@ -409,7 +409,8 @@ enum scrub_error_type PGBackend::be_compare_scrub_objects(
if (auth.size != candidate.size) {
if (error != CLEAN)
errorstream << ", ";
error = SHALLOW_ERROR;
if (error != DEEP_ERROR)
error = SHALLOW_ERROR;
Copy link
Contributor

Choose a reason for hiding this comment

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

There is a comment above that says that the shallow error here takes precedence because this can be seen by both kinds of scrubs. This is a bit odd, but I'd say remove this entire commit unless you are seeing a particular problem.

Copy link
Member

Choose a reason for hiding this comment

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

Hmm... making shallow take precedence doesn't make sense to me. Anybody remember why it says that? @athanatos ?

Copy link
Contributor

Choose a reason for hiding this comment

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

@liewegas In addition to your change at line 412 we should not set read_error in be_scan_list() on stat() errors and set the stat size to something invalid instead. Also, remove the comments at line 374 and at line 407.

This way a regular scrub will never get a DEEP_ERROR from this function, but a deep-scrub could possibly see a SHALLOW_ERROR if only stat() fails. This keeps the error counts from getting messed up.

@ghost ghost added bug-fix core labels Oct 16, 2015
@majianpeng
Copy link
Member Author

@tchaikov . Have you time to review this? Thanks!

@tchaikov tchaikov self-assigned this Nov 13, 2015
@liewegas
Copy link
Member

This needs rebase. And a run through the qa suite. (Ping us in #sepia if you need help using teuthology-openstack!)

@dzafman
Copy link
Contributor

dzafman commented Nov 20, 2015

I've got a rebase and includes addition of stat_error as pull request #6669

@tchaikov tchaikov removed their assignment Nov 23, 2015
@tchaikov
Copy link
Contributor

reviewed.

@liewegas
Copy link
Member

needs rebase?

@liewegas
Copy link
Member

nm, see @dzafman rebase

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