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: record prior_version for DELETE events #15649

Merged
merged 3 commits into from Jun 22, 2017

Conversation

Projects
None yet
2 participants
@liewegas
Copy link
Member

liewegas commented Jun 13, 2017

Leaving this blank is inaccurate. It also confuses the rewind
divergent log code, leading to behavior like

  ldpp_dout(dpp, 10) << __func__ << ": hoid " << hoid
		 << " prior_version or op type indicates creation,"
		 << " deleting"
		 << dendl;

Fixes: http://tracker.ceph.com/issues/20274
Signed-off-by: Sage Weil sage@redhat.com

@liewegas liewegas requested a review from jdurgin Jun 13, 2017

@liewegas

This comment has been minimized.

Copy link
Member Author

liewegas commented Jun 13, 2017

@jdurgin does this make sense? i'm not too familiar with the error events.

@liewegas liewegas changed the title osd/PrimaryLogPG: record prior_version for DELETE events RFC osd/PrimaryLogPG: record prior_version for DELETE events Jun 13, 2017

@jdurgin
Copy link
Member

jdurgin left a comment

That looks correct to record prior_version when available. Errors recorded where the obc doesn't exist will still hit the same case though. It looks like we should ignore error entries in the two conditions that compare their prior_version to eversion_t():

  1. pg_missing_set::add_next_event()
  2. PGLog::_merge_object_divergent_entries()

@liewegas liewegas force-pushed the liewegas:wip-20274 branch from b51cc60 to e95210c Jun 13, 2017

@liewegas

This comment has been minimized.

Copy link
Member Author

liewegas commented Jun 13, 2017

@jdurgin

This comment has been minimized.

Copy link
Member

jdurgin commented Jun 13, 2017

ah, I was looking at my branch. The tracker part is keeping a delta to determine what needs to be written to disk the next time the missing set is persisted. Since the error entry doesn't changed the missing set, we can skip that call for ERROR entries.

while (i != entries.end()) {
if (i->is_error()) {
ldpp_dout(dpp, 20) << __func__ << ": ignoring " << *i << dendl;
i = entries.erase(i);

This comment has been minimized.

Copy link
@jdurgin

jdurgin Jun 13, 2017

Member

isn't entries const?

@@ -3757,8 +3757,9 @@ class pg_missing_set : public pg_missing_const_i {
} else if (e.is_delete()) {
rm(e.soid, e.version);
}

tracker.changed(e.soid);
if (!e.is_error()) {

This comment has been minimized.

Copy link
@jdurgin

jdurgin Jun 13, 2017

Member

now that I look the only 2 callers of this already avoid adding ERROR entries, so this isn't needed

@liewegas

This comment has been minimized.

Copy link
Member Author

liewegas commented Jun 13, 2017

liewegas added some commits Jun 13, 2017

osd/osd_types: do not track ERROR change for missing set
Signed-off-by: Sage Weil <sage@redhat.com>
osd/PGLog: remove some dead code
Signed-off-by: Sage Weil <sage@redhat.com>

@liewegas liewegas force-pushed the liewegas:wip-20274 branch from 2524197 to b0919a2 Jun 13, 2017

@jdurgin

This comment has been minimized.

Copy link
Member

jdurgin commented Jun 14, 2017

'make check' error looks real

@liewegas liewegas force-pushed the liewegas:wip-20274 branch from b0919a2 to 986a31f Jun 14, 2017

@liewegas liewegas changed the title RFC osd/PrimaryLogPG: record prior_version for DELETE events DNM osd/PrimaryLogPG: record prior_version for DELETE events Jun 16, 2017

@liewegas liewegas added the needs-qa label Jun 16, 2017

@liewegas liewegas changed the title DNM osd/PrimaryLogPG: record prior_version for DELETE events osd/PrimaryLogPG: record prior_version for DELETE events Jun 20, 2017

osd/PGLog: ignore ERROR entires in _merge_object_divergent_entries
Sometimes ERROR log entries do not have prior_version.  Also,
they aren't actually updates and don't affect the object.

Fixes: http://tracker.ceph.com/issues/20274
Signed-off-by: Sage Weil <sage@redhat.com>

@liewegas liewegas force-pushed the liewegas:wip-20274 branch from 986a31f to 7879efd Jun 21, 2017

@liewegas liewegas merged commit dd8fed5 into ceph:master Jun 22, 2017

3 of 4 checks passed

make check make check failed
Details
Signed-off-by all commits in this PR are signed
Details
Unmodified Submodules submodules for project are unmodified
Details
arm64 make check arm64 make check succeeded
Details

@liewegas liewegas deleted the liewegas:wip-20274 branch Jun 22, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.