Skip to content

remove all metadata related#2426

Closed
il9ue wants to merge 2 commits intocockroachdb:masterfrom
il9ue:il9ue-patch-1
Closed

remove all metadata related#2426
il9ue wants to merge 2 commits intocockroachdb:masterfrom
il9ue:il9ue-patch-1

Conversation

@il9ue
Copy link
Copy Markdown
Contributor

@il9ue il9ue commented Sep 9, 2015

based on the comments about MakeRangeIDKey, added the remove all metadata involved with it. added the function to check any errors while deleting metadata.

ilque and others added 2 commits September 7, 2015 02:05
based on the comments about MakeRangeIDKey, added the remove all metadata involved with it. added the function to check any errors while deleting metadata.
@CLAassistant
Copy link
Copy Markdown

CLA assistant check
All committers of the pull request should sign our Contributor License Agreement in order to get your pull request merged.
1 out of 2 committers have signed the CLA.

✅ il9ue
❌ ilque


ilque seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account (for further information, click here).

@tamird
Copy link
Copy Markdown
Contributor

tamird commented Sep 9, 2015

@il9ue you can please update your existing PR? #2395. Also, this needs a rebase.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

there is a lot of duplication here, which you can eliminate using a slice of anonymous structs:

now := s.Clock().Now()
metadata := []struct{
  name string
  key    proto.Key
}{
  {"cache", keys.ResponseCacheKey(rangeID, nil)},
  {"logprefix", keys.RaftLogPrefix(rangeID)},
  {"hardstate", keys.RaftHardStateKey(rangeID)},
  ...
}
for _, metadatum := range metadata {
  if err := engine.MVCCDelete(s.engine, nil, metadatum.key, now, nil); err != nil {
    return util.Errorf("cannot delete %s key %s: %s", metadatum.name, metadatum.key, err)
  }
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Rather than enumerating all of these things, I think we could just construct a prefix that matches all RangeIDKeys.

@tamird
Copy link
Copy Markdown
Contributor

tamird commented Sep 9, 2015

This also needs tests.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

MVCCDelete deletes a single value but RaftLogPrefix identifies a range. This needs to use MVCCDeleteRange.

@bdarnell bdarnell mentioned this pull request Sep 9, 2015
9 tasks
@tamird
Copy link
Copy Markdown
Contributor

tamird commented Sep 21, 2015

Closing in favour #2395.

@tamird tamird closed this Sep 21, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants