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

Remove measurement from shard index on DROP #3002

Merged
merged 3 commits into from
Jun 16, 2015
Merged

Conversation

otoolep
Copy link
Contributor

@otoolep otoolep commented Jun 16, 2015

When a measurement is DROPped, its record in the shard's inverted index must also be deleted.

Fixes issue #2955.

@otoolep otoolep changed the title Recreate measurement fail Remove measurement from shard index on DROP Jun 16, 2015
@otoolep
Copy link
Contributor Author

otoolep commented Jun 16, 2015

@pauldix

@@ -294,6 +294,8 @@ func (s *Shard) deleteMeasurement(name string, seriesKeys []string) error {
return err
}

// Remove entry from shard index.
delete(s.measurementFields, name)
Copy link
Member

Choose a reason for hiding this comment

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

need to get the write mutex on the shard before doing this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, let me do that.

@otoolep otoolep force-pushed the recreate_measurement_fail branch from 3211b2e to 2f4b089 Compare June 16, 2015 19:15
@otoolep
Copy link
Contributor Author

otoolep commented Jun 16, 2015

Correct locking added.

@@ -294,6 +294,10 @@ func (s *Shard) deleteMeasurement(name string, seriesKeys []string) error {
return err
}

// Remove entry from shard index.
s.mu.Lock()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It doesn't seem necessary to lock for the entire function, so just doing it here.

Copy link
Member

Choose a reason for hiding this comment

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

it isn't that's right

@pauldix
Copy link
Member

pauldix commented Jun 16, 2015

+1

@otoolep
Copy link
Contributor Author

otoolep commented Jun 16, 2015

Thanks @pauldix -- merging now.

otoolep added a commit that referenced this pull request Jun 16, 2015
Remove measurement from shard index on DROP
@otoolep otoolep merged commit 572a43d into master Jun 16, 2015
@otoolep otoolep deleted the recreate_measurement_fail branch June 16, 2015 19:17
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.

2 participants