From 76c29053cf5ed01a35c7fede714cecf5fd235116 Mon Sep 17 00:00:00 2001 From: Philip O'Toole Date: Mon, 27 Apr 2015 14:19:24 -0700 Subject: [PATCH 1/3] Move drop series lock control into local function This change means that lock control can use the defer call, which means there is no chance the RLock will be left locked at function exit Previously this code was more complex as it managed locks manually, since the RLock must be released to allow the "drop series" broadcast message go through. --- server.go | 84 +++++++++++++++++++++++++++++-------------------------- 1 file changed, 45 insertions(+), 39 deletions(-) diff --git a/server.go b/server.go index b2fb22a9410..0cc128d072a 100644 --- a/server.go +++ b/server.go @@ -2499,59 +2499,65 @@ func (s *Server) executeDropMeasurementStatement(stmt *influxql.DropMeasurementS } func (s *Server) executeDropSeriesStatement(stmt *influxql.DropSeriesStatement, database string, user *User) *Result { - s.mu.RLock() + seriesByMeasurement, err := func() (map[string][]uint64, error) { + // Using a local function makes lock management foolproof. + s.mu.RLock() + defer s.mu.RUnlock() - seriesByMeasurement := make(map[string][]uint64) - // Handle the simple `DROP SERIES ` case. - if stmt.Source == nil && stmt.Condition == nil { - for _, db := range s.databases { - for _, m := range db.measurements { - if m.seriesByID[stmt.SeriesID] != nil { - seriesByMeasurement[m.Name] = []uint64{stmt.SeriesID} + seriesByMeasurement := make(map[string][]uint64) + // Handle the simple `DROP SERIES ` case. + if stmt.Source == nil && stmt.Condition == nil { + for _, db := range s.databases { + for _, m := range db.measurements { + if m.seriesByID[stmt.SeriesID] != nil { + seriesByMeasurement[m.Name] = []uint64{stmt.SeriesID} + } } } + + return seriesByMeasurement, nil } - s.mu.RUnlock() - return &Result{Err: s.DropSeries(database, seriesByMeasurement)} - } + // Handle the more complicated `DROP SERIES` with sources and/or conditions... - // Handle the more complicated `DROP SERIES` with sources and/or conditions... + // Find the database. + db := s.databases[database] + if db == nil { + return nil, ErrDatabaseNotFound(database) + } - // Find the database. - db := s.databases[database] - if db == nil { - s.mu.RUnlock() - return &Result{Err: ErrDatabaseNotFound(database)} - } + // Get the list of measurements we're interested in. + measurements, err := measurementsFromSourceOrDB(stmt.Source, db) + if err != nil { + return nil, err + } - // Get the list of measurements we're interested in. - measurements, err := measurementsFromSourceOrDB(stmt.Source, db) - if err != nil { - s.mu.RUnlock() - return &Result{Err: err} - } + for _, m := range measurements { + var ids seriesIDs + if stmt.Condition != nil { + // Get series IDs that match the WHERE clause. + filters := map[uint64]influxql.Expr{} + ids, _, _, err = m.walkWhereForSeriesIds(stmt.Condition, filters) + if err != nil { + return nil, err + } - for _, m := range measurements { - var ids seriesIDs - if stmt.Condition != nil { - // Get series IDs that match the WHERE clause. - filters := map[uint64]influxql.Expr{} - ids, _, _, err = m.walkWhereForSeriesIds(stmt.Condition, filters) - if err != nil { - return &Result{Err: err} + // TODO: check return of walkWhereForSeriesIds for fields + } else { + // No WHERE clause so get all series IDs for this measurement. + ids = m.seriesIDs } - // TODO: check return of walkWhereForSeriesIds for fields - } else { - // No WHERE clause so get all series IDs for this measurement. - ids = m.seriesIDs + seriesByMeasurement[m.Name] = ids + } - seriesByMeasurement[m.Name] = ids - } - s.mu.RUnlock() + return seriesByMeasurement, nil + }() + if err != nil { + return &Result{Err: err} + } return &Result{Err: s.DropSeries(database, seriesByMeasurement)} } From e78c481c018bd2242cb5c51ed48447b2b89fb7fc Mon Sep 17 00:00:00 2001 From: Philip O'Toole Date: Mon, 27 Apr 2015 14:21:46 -0700 Subject: [PATCH 2/3] Show features first in CHANGELOG --- CHANGELOG.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 938a72fbd14..f42e09e2e8e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,8 @@ ## v0.9.0-rc28 [unreleased] +### Features +- [#2410](https://github.com/influxdb/influxdb/pull/2410) Allow configuration of Raft timers + ### Bugfixes - [#2374](https://github.com/influxdb/influxdb/issues/2374): Two different panics during SELECT percentile - [#2404](https://github.com/influxdb/influxdb/pull/2404): Mean and percentile function fixes @@ -12,9 +15,6 @@ - [#2429](https://github.com/influxdb/influxdb/pull/2429): Ensure no field value is null. - [#2431](https://github.com/influxdb/influxdb/pull/2431): Always append shard path in diags. Thanks @marcosnils -### Features -- [#2410](https://github.com/influxdb/influxdb/pull/2410) Allow configuration of Raft timers - ## v0.9.0-rc27 [04-23-2015] ### Features From 26348534b962b696aa6e40a379875a6c8dc2a0e4 Mon Sep 17 00:00:00 2001 From: Philip O'Toole Date: Mon, 27 Apr 2015 14:22:29 -0700 Subject: [PATCH 3/3] Update CHANGELOG --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f42e09e2e8e..5bff0adccdb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ - [#2426](https://github.com/influxdb/influxdb/pull/2426): Fix race condition around listener address in Graphite server. - [#2429](https://github.com/influxdb/influxdb/pull/2429): Ensure no field value is null. - [#2431](https://github.com/influxdb/influxdb/pull/2431): Always append shard path in diags. Thanks @marcosnils +- [#2441](https://github.com/influxdb/influxdb/pull/2441): Correctly release server RLock during "drop series". ## v0.9.0-rc27 [04-23-2015]