From 89a587e8656b91ded0483b6cbb7befb7f47d3358 Mon Sep 17 00:00:00 2001 From: Mark Rushakoff Date: Tue, 3 Jan 2017 14:06:39 -0800 Subject: [PATCH] Use one atomic operation in (*Cache).decreaseSize The previous implementation was susceptible to a race condition (of correctness) since c.decreaseSize is called without a lock in (*Cache).WriteMulti. There were already tests which asserted the correctness of the result of decreaseSize, so no tests were added or modified. --- CHANGELOG.md | 1 + tsdb/engine/tsm1/cache.go | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0cc86a6f24f..f506cacb7f4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,7 @@ The stress tool `influx_stress` will be removed in a subsequent release. We reco ### Bugfixes +- [#7786](https://github.com/influxdata/influxdb/pull/7786): Fix potential race condition in correctness of tsm1_cache memBytes statistic. - [#7784](https://github.com/influxdata/influxdb/pull/7784): Fix broken error return on meta client's UpdateUser and DropContinuousQuery methods. - [#7741](https://github.com/influxdata/influxdb/pull/7741): Fix string quoting and significantly improve performance of `influx_inspect export`. - [#7698](https://github.com/influxdata/influxdb/pull/7698): CLI was caching db/rp for insert into statements. diff --git a/tsdb/engine/tsm1/cache.go b/tsdb/engine/tsm1/cache.go index 297bb8d4767..f4c5244a9e9 100644 --- a/tsdb/engine/tsm1/cache.go +++ b/tsdb/engine/tsm1/cache.go @@ -423,8 +423,8 @@ func (c *Cache) increaseSize(delta uint64) { // decreaseSize decreases size by delta. func (c *Cache) decreaseSize(delta uint64) { - size := atomic.LoadUint64(&c.size) - atomic.StoreUint64(&c.size, size-delta) + // Per sync/atomic docs, bit-flip delta minus one to perform subtraction within AddUint64. + atomic.AddUint64(&c.size, ^(delta - 1)) } // MaxSize returns the maximum number of bytes the cache may consume.