From 07c6e8000c6adfcd5a21524d9c96f12b07397b5d Mon Sep 17 00:00:00 2001 From: mcrakhman Date: Wed, 8 Mar 2023 22:07:44 +0100 Subject: [PATCH 01/12] Add ocache tryClose --- app/ocache/ocache.go | 325 ++++++++++++++++++++------------------ app/ocache/ocache_test.go | 41 ++--- 2 files changed, 197 insertions(+), 169 deletions(-) diff --git a/app/ocache/ocache.go b/app/ocache/ocache.go index df19b220..eb67f120 100644 --- a/app/ocache/ocache.go +++ b/app/ocache/ocache.go @@ -4,6 +4,7 @@ import ( "context" "errors" "github.com/anytypeio/any-sync/app/logger" + "github.com/anytypeio/any-sync/util/slice" "go.uber.org/zap" "sync" "time" @@ -44,12 +45,6 @@ var WithGCPeriod = func(gcPeriod time.Duration) Option { } } -var WithRefCounter = func(enable bool) Option { - return func(cache *oCache) { - cache.refCounter = enable - } -} - func New(loadFunc LoadFunc, opts ...Option) OCache { c := &oCache{ data: make(map[string]*entry), @@ -73,33 +68,117 @@ func New(loadFunc LoadFunc, opts ...Option) OCache { type Object interface { Close() (err error) + TryClose() (res bool, err error) } -type ObjectLocker interface { - Object - Locked() bool -} +type entryState int -type ObjectLastUsage interface { - LastUsage() time.Time -} +const ( + entryStateLoading = iota + entryStateActive + entryStateClosing + entryStateClosed +) type entry struct { id string + state entryState lastUsage time.Time - refCount uint32 - isClosing bool load chan struct{} loadErr error value Object close chan struct{} + mx sync.Mutex } -func (e *entry) locked() bool { - if locker, ok := e.value.(ObjectLocker); ok { - return locker.Locked() +func newEntry(id string, value Object, state entryState) *entry { + return &entry{ + id: id, + load: make(chan struct{}), + lastUsage: time.Now(), + state: state, + value: value, } - return false +} + +func (e *entry) getState() entryState { + e.mx.Lock() + defer e.mx.Unlock() + return e.state +} + +func (e *entry) isClosing() bool { + e.mx.Lock() + defer e.mx.Unlock() + return e.state == entryStateClosed || e.state == entryStateClosing +} + +func (e *entry) waitLoad(ctx context.Context, id string) (value Object, err error) { + select { + case <-ctx.Done(): + log.DebugCtx(ctx, "ctx done while waiting on object load", zap.String("id", id)) + return nil, ctx.Err() + case <-e.load: + return e.value, e.loadErr + } +} + +func (e *entry) waitClose(ctx context.Context, id string) (res bool, err error) { + e.mx.Lock() + switch e.state { + case entryStateClosing: + waitCh := e.close + e.mx.Unlock() + select { + case <-ctx.Done(): + log.DebugCtx(ctx, "ctx done while waiting on object close", zap.String("id", id)) + return false, ctx.Err() + case <-waitCh: + return true, nil + } + case entryStateClosed: + e.mx.Unlock() + return true, nil + default: + e.mx.Unlock() + return false, nil + } +} + +func (e *entry) setClosing(wait bool) (prevState entryState) { + e.mx.Lock() + prevState = e.state + if e.state == entryStateClosing { + waitCh := e.close + e.mx.Unlock() + if !wait { + return + } + <-waitCh + e.mx.Lock() + } + if e.state != entryStateClosed { + e.state = entryStateClosing + e.close = make(chan struct{}) + } + e.mx.Unlock() + return +} + +func (e *entry) setActive(chClose bool) { + e.mx.Lock() + defer e.mx.Unlock() + if chClose { + close(e.close) + } + e.state = entryStateActive +} + +func (e *entry) setClosed() { + e.mx.Lock() + defer e.mx.Unlock() + close(e.close) + e.state = entryStateClosed } type OCache interface { @@ -116,10 +195,6 @@ type OCache interface { // Add adds new object to cache // Returns error when object exists Add(id string, value Object) (err error) - // Release decreases the refs counter - Release(id string) bool - // Reset sets refs counter to 0 - Reset(id string) bool // Remove closes and removes object Remove(id string) (ok bool, err error) // ForEach iterates over all loaded objects, breaks when callback returns false @@ -134,17 +209,16 @@ type OCache interface { } type oCache struct { - mu sync.Mutex - data map[string]*entry - loadFunc LoadFunc - timeNow func() time.Time - ttl time.Duration - gc time.Duration - closed bool - closeCh chan struct{} - log *zap.SugaredLogger - metrics *metrics - refCounter bool + mu sync.Mutex + data map[string]*entry + loadFunc LoadFunc + timeNow func() time.Time + ttl time.Duration + gc time.Duration + closed bool + closeCh chan struct{} + log *zap.SugaredLogger + metrics *metrics } func (c *oCache) Get(ctx context.Context, id string) (value Object, err error) { @@ -160,69 +234,46 @@ Load: return nil, ErrClosed } if e, ok = c.data[id]; !ok { + e = newEntry(id, nil, entryStateLoading) load = true - e = &entry{ - id: id, - load: make(chan struct{}), - } c.data[id] = e } - closing := e.isClosing - if !e.isClosing { - e.lastUsage = c.timeNow() - if c.refCounter { - e.refCount++ - } - } c.mu.Unlock() - if closing { - select { - case <-ctx.Done(): - log.DebugCtx(ctx, "ctx done while waiting on object close", zap.String("id", id)) - return nil, ctx.Err() - case <-e.close: - goto Load - } + reload, err := e.waitClose(ctx, id) + if err != nil { + return nil, err + } + if reload { + goto Load } - if load { go c.load(ctx, id, e) } - if c.metrics != nil { - if load { - c.metrics.miss.Inc() - } else { - c.metrics.hit.Inc() - } + c.metricsGet(!load) + return e.waitLoad(ctx, id) +} + +func (c *oCache) metricsGet(hit bool) { + if c.metrics == nil { + return } - select { - case <-ctx.Done(): - log.DebugCtx(ctx, "ctx done while waiting on object load", zap.String("id", id)) - return nil, ctx.Err() - case <-e.load: + if hit { + c.metrics.hit.Inc() + } else { + c.metrics.miss.Inc() } - return e.value, e.loadErr } func (c *oCache) Pick(ctx context.Context, id string) (value Object, err error) { c.mu.Lock() val, ok := c.data[id] - if !ok || val.isClosing { + if !ok || val.isClosing() { c.mu.Unlock() return nil, ErrNotExists } c.mu.Unlock() - - if c.metrics != nil { - c.metrics.hit.Inc() - } - - select { - case <-ctx.Done(): - return nil, ctx.Err() - case <-val.load: - return val.value, val.loadErr - } + c.metricsGet(true) + return val.waitLoad(ctx, id) } func (c *oCache) load(ctx context.Context, id string, e *entry) { @@ -236,37 +287,10 @@ func (c *oCache) load(ctx context.Context, id string, e *entry) { delete(c.data, id) } else { e.value = value + e.setActive(false) } } -func (c *oCache) Release(id string) bool { - c.mu.Lock() - defer c.mu.Unlock() - if c.closed { - return false - } - if e, ok := c.data[id]; ok { - if c.refCounter && e.refCount > 0 { - e.refCount-- - return true - } - } - return false -} - -func (c *oCache) Reset(id string) bool { - c.mu.Lock() - defer c.mu.Unlock() - if c.closed { - return false - } - if e, ok := c.data[id]; ok { - e.refCount = 0 - return true - } - return false -} - func (c *oCache) Remove(id string) (ok bool, err error) { c.mu.Lock() if c.closed { @@ -274,25 +298,33 @@ func (c *oCache) Remove(id string) (ok bool, err error) { err = ErrClosed return } - var e *entry - e, ok = c.data[id] - if !ok || e.isClosing { + e, ok := c.data[id] + if !ok { c.mu.Unlock() return } - e.isClosing = true - e.close = make(chan struct{}) c.mu.Unlock() + return c.remove(e, true) +} +func (c *oCache) remove(e *entry, remData bool) (ok bool, err error) { <-e.load - if e.value != nil { + if e.value == nil { + return false, ErrNotExists + } + prevState := e.setClosing(true) + if prevState == entryStateActive { err = e.value.Close() + e.setClosed() + } + if !remData { + return } c.mu.Lock() - close(e.close) - delete(c.data, e.id) + if prevState == entryStateActive { + delete(c.data, e.id) + } c.mu.Unlock() - return } @@ -314,13 +346,7 @@ func (c *oCache) Add(id string, value Object) (err error) { if _, ok := c.data[id]; ok { return ErrExists } - e := &entry{ - id: id, - lastUsage: time.Now(), - refCount: 0, - load: make(chan struct{}), - value: value, - } + e := newEntry(id, value, entryStateActive) close(e.load) c.data[id] = e return @@ -332,7 +358,7 @@ func (c *oCache) ForEach(f func(obj Object) (isContinue bool)) { for _, v := range c.data { select { case <-v.load: - if v.value != nil && !v.isClosing { + if v.value != nil && !v.isClosing() { objects = append(objects, v.value) } default: @@ -368,15 +394,10 @@ func (c *oCache) GC() { deadline := c.timeNow().Add(-c.ttl) var toClose []*entry for _, e := range c.data { - if e.isClosing { + if e.getState() != entryStateActive { continue } - lu := e.lastUsage - if lug, ok := e.value.(ObjectLastUsage); ok { - lu = lug.LastUsage() - } - if !e.locked() && e.refCount <= 0 && lu.Before(deadline) { - e.isClosing = true + if e.lastUsage.Before(deadline) { e.close = make(chan struct{}) toClose = append(toClose, e) } @@ -384,21 +405,33 @@ func (c *oCache) GC() { size := len(c.data) c.mu.Unlock() - for _, e := range toClose { - <-e.load - if e.value != nil { - if err := e.value.Close(); err != nil { - c.log.With("object_id", e.id).Warnf("GC: object close error: %v", err) - } + for idx, e := range toClose { + prevState := e.setClosing(false) + if prevState == entryStateClosing || prevState == entryStateClosed { + toClose[idx] = nil + continue + } + ok, err := e.value.TryClose() + if !ok { + e.setActive(true) + toClose[idx] = nil + continue + } else { + e.setClosed() + } + if err != nil { + c.log.With("object_id", e.id).Warnf("GC: object close error: %v", err) } } + toClose = slice.DiscardFromSlice(toClose, func(e *entry) bool { + return e == nil + }) c.log.Infof("GC: removed %d; cache size: %d", len(toClose), size) if len(toClose) > 0 && c.metrics != nil { c.metrics.gc.Add(float64(len(toClose))) } c.mu.Lock() for _, e := range toClose { - close(e.close) delete(c.data, e.id) } c.mu.Unlock() @@ -418,25 +451,15 @@ func (c *oCache) Close() (err error) { } c.closed = true close(c.closeCh) - var toClose, alreadyClosing []*entry + var toClose []*entry for _, e := range c.data { - if e.isClosing { - alreadyClosing = append(alreadyClosing, e) - } else { - toClose = append(toClose, e) - } + toClose = append(toClose, e) } c.mu.Unlock() for _, e := range toClose { - <-e.load - if e.value != nil { - if clErr := e.value.Close(); clErr != nil { - c.log.With("object_id", e.id).Warnf("cache close: object close error: %v", clErr) - } + if _, err := c.remove(e, false); err != ErrNotExists { + c.log.With("object_id", e.id).Warnf("cache close: object close error: %v", err) } } - for _, e := range alreadyClosing { - <-e.close - } return nil } diff --git a/app/ocache/ocache_test.go b/app/ocache/ocache_test.go index 54034141..d32b01f7 100644 --- a/app/ocache/ocache_test.go +++ b/app/ocache/ocache_test.go @@ -12,15 +12,17 @@ import ( ) type testObject struct { - name string - closeErr error - closeCh chan struct{} + name string + closeErr error + closeCh chan struct{} + tryReturn bool } -func NewTestObject(name string, closeCh chan struct{}) *testObject { +func NewTestObject(name string, tryReturn bool, closeCh chan struct{}) *testObject { return &testObject{ - name: name, - closeCh: closeCh, + name: name, + closeCh: closeCh, + tryReturn: tryReturn, } } @@ -31,6 +33,14 @@ func (t *testObject) Close() (err error) { return t.closeErr } +func (t *testObject) TryClose() (res bool, err error) { + if t.closeCh != nil { + <-t.closeCh + return true, t.closeErr + } + return t.tryReturn, nil +} + func TestOCache_Get(t *testing.T) { t.Run("successful", func(t *testing.T) { c := New(func(ctx context.Context, id string) (value Object, err error) { @@ -118,8 +128,8 @@ func TestOCache_Get(t *testing.T) { func TestOCache_GC(t *testing.T) { t.Run("test without close wait", func(t *testing.T) { c := New(func(ctx context.Context, id string) (value Object, err error) { - return &testObject{name: id}, nil - }, WithTTL(time.Millisecond*10), WithRefCounter(true)) + return NewTestObject(id, true, nil), nil + }, WithTTL(time.Millisecond*10)) val, err := c.Get(context.TODO(), "id") require.NoError(t, err) require.NotNil(t, val) @@ -128,24 +138,19 @@ func TestOCache_GC(t *testing.T) { assert.Equal(t, 1, c.Len()) time.Sleep(time.Millisecond * 30) c.GC() - assert.Equal(t, 1, c.Len()) - assert.True(t, c.Release("id")) - c.GC() assert.Equal(t, 0, c.Len()) - assert.False(t, c.Release("id")) }) t.Run("test with close wait", func(t *testing.T) { closeCh := make(chan struct{}) getCh := make(chan struct{}) c := New(func(ctx context.Context, id string) (value Object, err error) { - return NewTestObject(id, closeCh), nil - }, WithTTL(time.Millisecond*10), WithRefCounter(true)) + return NewTestObject(id, true, closeCh), nil + }, WithTTL(time.Millisecond*10)) val, err := c.Get(context.TODO(), "id") require.NoError(t, err) require.NotNil(t, val) assert.Equal(t, 1, c.Len()) - assert.True(t, c.Release("id")) // making ttl pass time.Sleep(time.Millisecond * 40) // first gc will be run after 20 secs, so calling it manually @@ -160,9 +165,9 @@ func TestOCache_GC(t *testing.T) { events = append(events, "get") close(getCh) }() - events = append(events, "close") // sleeping to make sure that Get is called time.Sleep(time.Millisecond * 40) + events = append(events, "close") close(closeCh) <-getCh @@ -175,7 +180,7 @@ func Test_OCache_Remove(t *testing.T) { getCh := make(chan struct{}) c := New(func(ctx context.Context, id string) (value Object, err error) { - return NewTestObject(id, closeCh), nil + return NewTestObject(id, false, closeCh), nil }, WithTTL(time.Millisecond*10)) val, err := c.Get(context.TODO(), "id") require.NoError(t, err) @@ -196,9 +201,9 @@ func Test_OCache_Remove(t *testing.T) { events = append(events, "get") close(getCh) }() - events = append(events, "close") // sleeping to make sure that Get is called time.Sleep(time.Millisecond * 40) + events = append(events, "close") close(closeCh) <-getCh From 65496d247ff8237b926d40acdf878540241198ba Mon Sep 17 00:00:00 2001 From: mcrakhman Date: Wed, 8 Mar 2023 22:52:43 +0100 Subject: [PATCH 02/12] Test returning to cache after tryclose --- app/ocache/ocache_test.go | 64 ++++++++++++++++++++++++++++++++++----- 1 file changed, 57 insertions(+), 7 deletions(-) diff --git a/app/ocache/ocache_test.go b/app/ocache/ocache_test.go index d32b01f7..8d963214 100644 --- a/app/ocache/ocache_test.go +++ b/app/ocache/ocache_test.go @@ -3,6 +3,7 @@ package ocache import ( "context" "errors" + "sync" "sync/atomic" "testing" "time" @@ -12,10 +13,12 @@ import ( ) type testObject struct { - name string - closeErr error - closeCh chan struct{} - tryReturn bool + name string + closeErr error + closeCh chan struct{} + tryReturn bool + closeCalled bool + tryCloseCalled bool } func NewTestObject(name string, tryReturn bool, closeCh chan struct{}) *testObject { @@ -27,6 +30,7 @@ func NewTestObject(name string, tryReturn bool, closeCh chan struct{}) *testObje } func (t *testObject) Close() (err error) { + t.closeCalled = true if t.closeCh != nil { <-t.closeCh } @@ -34,6 +38,7 @@ func (t *testObject) Close() (err error) { } func (t *testObject) TryClose() (res bool, err error) { + t.tryCloseCalled = true if t.closeCh != nil { <-t.closeCh return true, t.closeErr @@ -126,7 +131,7 @@ func TestOCache_Get(t *testing.T) { } func TestOCache_GC(t *testing.T) { - t.Run("test without close wait", func(t *testing.T) { + t.Run("test gc expired object", func(t *testing.T) { c := New(func(ctx context.Context, id string) (value Object, err error) { return NewTestObject(id, true, nil), nil }, WithTTL(time.Millisecond*10)) @@ -140,7 +145,7 @@ func TestOCache_GC(t *testing.T) { c.GC() assert.Equal(t, 0, c.Len()) }) - t.Run("test with close wait", func(t *testing.T) { + t.Run("test gc tryClose true, close before get", func(t *testing.T) { closeCh := make(chan struct{}) getCh := make(chan struct{}) @@ -173,15 +178,60 @@ func TestOCache_GC(t *testing.T) { <-getCh require.Equal(t, []string{"close", "get"}, events) }) + t.Run("test gc tryClose false, many get", func(t *testing.T) { + timesCalled := &atomic.Int32{} + obj := NewTestObject("id", false, nil) + c := New(func(ctx context.Context, id string) (value Object, err error) { + timesCalled.Add(1) + return obj, nil + }, WithTTL(time.Millisecond*10)) + + val, err := c.Get(context.TODO(), "id") + require.NoError(t, err) + require.NotNil(t, val) + assert.Equal(t, 1, c.Len()) + // making ttl pass + time.Sleep(time.Millisecond * 40) + // first gc will be run after 20 secs, so calling it manually + begin := make(chan struct{}) + wg := sync.WaitGroup{} + once := sync.Once{} + + wg.Add(1) + go func() { + <-begin + c.GC() + wg.Done() + }() + + for i := 0; i < 5; i++ { + wg.Add(1) + go func(i int) { + once.Do(func() { + close(begin) + }) + if i > 0 { + time.Sleep(time.Duration(i) * time.Millisecond) + } + _, err := c.Get(context.TODO(), "id") + require.NoError(t, err) + wg.Done() + }(i) + } + require.NoError(t, err) + wg.Wait() + require.Equal(t, timesCalled.Load(), int32(1)) + require.True(t, obj.tryCloseCalled) + }) } func Test_OCache_Remove(t *testing.T) { closeCh := make(chan struct{}) getCh := make(chan struct{}) - c := New(func(ctx context.Context, id string) (value Object, err error) { return NewTestObject(id, false, closeCh), nil }, WithTTL(time.Millisecond*10)) + val, err := c.Get(context.TODO(), "id") require.NoError(t, err) require.NotNil(t, val) From 9836caf2cb1127a0e94cb54cd8b9ed155957708d Mon Sep 17 00:00:00 2001 From: mcrakhman Date: Thu, 9 Mar 2023 09:06:02 +0100 Subject: [PATCH 03/12] More tests and split entry --- app/ocache/entry.go | 120 ++++++++++++++++++++++++++++++++++++++ app/ocache/ocache.go | 118 ++----------------------------------- app/ocache/ocache_test.go | 51 +++++++++++++--- 3 files changed, 168 insertions(+), 121 deletions(-) create mode 100644 app/ocache/entry.go diff --git a/app/ocache/entry.go b/app/ocache/entry.go new file mode 100644 index 00000000..ad985fa0 --- /dev/null +++ b/app/ocache/entry.go @@ -0,0 +1,120 @@ +package ocache + +import ( + "context" + "go.uber.org/zap" + "sync" + "time" +) + +type entryState int + +const ( + entryStateLoading = iota + entryStateActive + entryStateClosing + entryStateClosed +) + +type entry struct { + id string + state entryState + lastUsage time.Time + load chan struct{} + loadErr error + value Object + close chan struct{} + mx sync.Mutex +} + +func newEntry(id string, value Object, state entryState) *entry { + return &entry{ + id: id, + load: make(chan struct{}), + lastUsage: time.Now(), + state: state, + value: value, + } +} + +func (e *entry) getState() entryState { + e.mx.Lock() + defer e.mx.Unlock() + return e.state +} + +func (e *entry) isClosing() bool { + e.mx.Lock() + defer e.mx.Unlock() + return e.state == entryStateClosed || e.state == entryStateClosing +} + +func (e *entry) waitLoad(ctx context.Context, id string) (value Object, err error) { + select { + case <-ctx.Done(): + log.DebugCtx(ctx, "ctx done while waiting on object load", zap.String("id", id)) + return nil, ctx.Err() + case <-e.load: + return e.value, e.loadErr + } +} + +func (e *entry) waitClose(ctx context.Context, id string) (res bool, err error) { + e.mx.Lock() + switch e.state { + case entryStateClosing: + waitCh := e.close + e.mx.Unlock() + select { + case <-ctx.Done(): + log.DebugCtx(ctx, "ctx done while waiting on object close", zap.String("id", id)) + return false, ctx.Err() + case <-waitCh: + return true, nil + } + case entryStateClosed: + e.mx.Unlock() + return true, nil + default: + e.mx.Unlock() + return false, nil + } +} + +func (e *entry) setClosing(wait bool) (prevState, curState entryState) { + e.mx.Lock() + prevState = e.state + curState = e.state + if e.state == entryStateClosing { + waitCh := e.close + e.mx.Unlock() + if !wait { + return + } + <-waitCh + e.mx.Lock() + } + if e.state != entryStateClosed { + e.state = entryStateClosing + e.close = make(chan struct{}) + } + curState = e.state + e.mx.Unlock() + return +} + +func (e *entry) setActive(chClose bool) { + e.mx.Lock() + defer e.mx.Unlock() + if chClose { + close(e.close) + } + e.state = entryStateActive +} + +func (e *entry) setClosed() { + e.mx.Lock() + defer e.mx.Unlock() + close(e.close) + e.state = entryStateClosed +} diff --git a/app/ocache/ocache.go b/app/ocache/ocache.go index eb67f120..4599f789 100644 --- a/app/ocache/ocache.go +++ b/app/ocache/ocache.go @@ -71,116 +71,6 @@ type Object interface { TryClose() (res bool, err error) } -type entryState int - -const ( - entryStateLoading = iota - entryStateActive - entryStateClosing - entryStateClosed -) - -type entry struct { - id string - state entryState - lastUsage time.Time - load chan struct{} - loadErr error - value Object - close chan struct{} - mx sync.Mutex -} - -func newEntry(id string, value Object, state entryState) *entry { - return &entry{ - id: id, - load: make(chan struct{}), - lastUsage: time.Now(), - state: state, - value: value, - } -} - -func (e *entry) getState() entryState { - e.mx.Lock() - defer e.mx.Unlock() - return e.state -} - -func (e *entry) isClosing() bool { - e.mx.Lock() - defer e.mx.Unlock() - return e.state == entryStateClosed || e.state == entryStateClosing -} - -func (e *entry) waitLoad(ctx context.Context, id string) (value Object, err error) { - select { - case <-ctx.Done(): - log.DebugCtx(ctx, "ctx done while waiting on object load", zap.String("id", id)) - return nil, ctx.Err() - case <-e.load: - return e.value, e.loadErr - } -} - -func (e *entry) waitClose(ctx context.Context, id string) (res bool, err error) { - e.mx.Lock() - switch e.state { - case entryStateClosing: - waitCh := e.close - e.mx.Unlock() - select { - case <-ctx.Done(): - log.DebugCtx(ctx, "ctx done while waiting on object close", zap.String("id", id)) - return false, ctx.Err() - case <-waitCh: - return true, nil - } - case entryStateClosed: - e.mx.Unlock() - return true, nil - default: - e.mx.Unlock() - return false, nil - } -} - -func (e *entry) setClosing(wait bool) (prevState entryState) { - e.mx.Lock() - prevState = e.state - if e.state == entryStateClosing { - waitCh := e.close - e.mx.Unlock() - if !wait { - return - } - <-waitCh - e.mx.Lock() - } - if e.state != entryStateClosed { - e.state = entryStateClosing - e.close = make(chan struct{}) - } - e.mx.Unlock() - return -} - -func (e *entry) setActive(chClose bool) { - e.mx.Lock() - defer e.mx.Unlock() - if chClose { - close(e.close) - } - e.state = entryStateActive -} - -func (e *entry) setClosed() { - e.mx.Lock() - defer e.mx.Unlock() - close(e.close) - e.state = entryStateClosed -} - type OCache interface { // DoLockedIfNotExists does an action if the object with id is not in cache // under a global lock, this will prevent a race which otherwise occurs @@ -312,8 +202,8 @@ func (c *oCache) remove(e *entry, remData bool) (ok bool, err error) { if e.value == nil { return false, ErrNotExists } - prevState := e.setClosing(true) - if prevState == entryStateActive { + _, curState := e.setClosing(true) + if curState == entryStateClosing { err = e.value.Close() e.setClosed() } @@ -321,7 +211,7 @@ func (c *oCache) remove(e *entry, remData bool) (ok bool, err error) { return } c.mu.Lock() - if prevState == entryStateActive { + if curState == entryStateClosing { delete(c.data, e.id) } c.mu.Unlock() @@ -406,7 +296,7 @@ func (c *oCache) GC() { c.mu.Unlock() for idx, e := range toClose { - prevState := e.setClosing(false) + prevState, _ := e.setClosing(false) if prevState == entryStateClosing || prevState == entryStateClosed { toClose[idx] = nil continue diff --git a/app/ocache/ocache_test.go b/app/ocache/ocache_test.go index 8d963214..43773184 100644 --- a/app/ocache/ocache_test.go +++ b/app/ocache/ocache_test.go @@ -3,6 +3,7 @@ package ocache import ( "context" "errors" + "fmt" "sync" "sync/atomic" "testing" @@ -178,7 +179,7 @@ func TestOCache_GC(t *testing.T) { <-getCh require.Equal(t, []string{"close", "get"}, events) }) - t.Run("test gc tryClose false, many get", func(t *testing.T) { + t.Run("test gc tryClose false, many parallel get", func(t *testing.T) { timesCalled := &atomic.Int32{} obj := NewTestObject("id", false, nil) c := New(func(ctx context.Context, id string) (value Object, err error) { @@ -190,9 +191,7 @@ func TestOCache_GC(t *testing.T) { require.NoError(t, err) require.NotNil(t, val) assert.Equal(t, 1, c.Len()) - // making ttl pass time.Sleep(time.Millisecond * 40) - // first gc will be run after 20 secs, so calling it manually begin := make(chan struct{}) wg := sync.WaitGroup{} once := sync.Once{} @@ -203,15 +202,14 @@ func TestOCache_GC(t *testing.T) { c.GC() wg.Done() }() - - for i := 0; i < 5; i++ { + for i := 0; i < 50; i++ { wg.Add(1) go func(i int) { once.Do(func() { close(begin) }) - if i > 0 { - time.Sleep(time.Duration(i) * time.Millisecond) + if i%2 != 0 { + time.Sleep(time.Millisecond) } _, err := c.Get(context.TODO(), "id") require.NoError(t, err) @@ -223,6 +221,45 @@ func TestOCache_GC(t *testing.T) { require.Equal(t, timesCalled.Load(), int32(1)) require.True(t, obj.tryCloseCalled) }) + t.Run("test gc tryClose different, many objects", func(t *testing.T) { + tryCloseIds := make(map[string]bool) + called := make(map[string]int) + max := 1000 + getId := func(i int) string { + return fmt.Sprintf("id%d", i) + } + for i := 0; i < max; i++ { + if i%2 == 1 { + tryCloseIds[getId(i)] = true + } else { + tryCloseIds[getId(i)] = false + } + } + c := New(func(ctx context.Context, id string) (value Object, err error) { + called[id] = called[id] + 1 + return NewTestObject(id, tryCloseIds[id], nil), nil + }, WithTTL(time.Millisecond*10)) + + for i := 0; i < max; i++ { + val, err := c.Get(context.TODO(), getId(i)) + require.NoError(t, err) + require.NotNil(t, val) + } + assert.Equal(t, max, c.Len()) + time.Sleep(time.Millisecond * 40) + c.GC() + for i := 0; i < max; i++ { + val, err := c.Get(context.TODO(), getId(i)) + require.NoError(t, err) + require.NotNil(t, val) + } + for i := 0; i < max; i++ { + val, err := c.Get(context.TODO(), getId(i)) + require.NoError(t, err) + require.NotNil(t, val) + require.Equal(t, called[getId(i)], i%2+1) + } + }) } func Test_OCache_Remove(t *testing.T) { From 4fb34fcb7ca98aabe2b4c29ad3e2458c587dc78a Mon Sep 17 00:00:00 2001 From: mcrakhman Date: Thu, 9 Mar 2023 09:38:45 +0100 Subject: [PATCH 04/12] Readability improvements --- app/ocache/entry.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/ocache/entry.go b/app/ocache/entry.go index ad985fa0..fbe60e26 100644 --- a/app/ocache/entry.go +++ b/app/ocache/entry.go @@ -37,10 +37,10 @@ func newEntry(id string, value Object, state entryState) *entry { } } -func (e *entry) getState() entryState { +func (e *entry) isActive() bool { e.mx.Lock() defer e.mx.Unlock() - return e.state + return e.state == entryStateActive } func (e *entry) isClosing() bool { From 63ba0a9fab3b40c05bae6a571a5f8d8fba5eb284 Mon Sep 17 00:00:00 2001 From: mcrakhman Date: Thu, 9 Mar 2023 20:25:01 +0100 Subject: [PATCH 05/12] Update cache logic to remove from entries on close --- app/ocache/ocache.go | 90 ++++++++++++++++++++------------------------ 1 file changed, 40 insertions(+), 50 deletions(-) diff --git a/app/ocache/ocache.go b/app/ocache/ocache.go index 4599f789..931cb431 100644 --- a/app/ocache/ocache.go +++ b/app/ocache/ocache.go @@ -4,7 +4,6 @@ import ( "context" "errors" "github.com/anytypeio/any-sync/app/logger" - "github.com/anytypeio/any-sync/util/slice" "go.uber.org/zap" "sync" "time" @@ -143,17 +142,6 @@ Load: return e.waitLoad(ctx, id) } -func (c *oCache) metricsGet(hit bool) { - if c.metrics == nil { - return - } - if hit { - c.metrics.hit.Inc() - } else { - c.metrics.miss.Inc() - } -} - func (c *oCache) Pick(ctx context.Context, id string) (value Object, err error) { c.mu.Lock() val, ok := c.data[id] @@ -191,13 +179,13 @@ func (c *oCache) Remove(id string) (ok bool, err error) { e, ok := c.data[id] if !ok { c.mu.Unlock() - return + return false, ErrNotExists } c.mu.Unlock() - return c.remove(e, true) + return c.remove(e) } -func (c *oCache) remove(e *entry, remData bool) (ok bool, err error) { +func (c *oCache) remove(e *entry) (ok bool, err error) { <-e.load if e.value == nil { return false, ErrNotExists @@ -205,16 +193,11 @@ func (c *oCache) remove(e *entry, remData bool) (ok bool, err error) { _, curState := e.setClosing(true) if curState == entryStateClosing { err = e.value.Close() + c.mu.Lock() e.setClosed() - } - if !remData { - return - } - c.mu.Lock() - if curState == entryStateClosing { delete(c.data, e.id) + c.mu.Unlock() } - c.mu.Unlock() return } @@ -284,47 +267,35 @@ func (c *oCache) GC() { deadline := c.timeNow().Add(-c.ttl) var toClose []*entry for _, e := range c.data { - if e.getState() != entryStateActive { - continue - } - if e.lastUsage.Before(deadline) { + if e.isActive() && e.lastUsage.Before(deadline) { e.close = make(chan struct{}) toClose = append(toClose, e) } } size := len(c.data) c.mu.Unlock() - - for idx, e := range toClose { + closedNum := 0 + for _, e := range toClose { prevState, _ := e.setClosing(false) if prevState == entryStateClosing || prevState == entryStateClosed { - toClose[idx] = nil continue } - ok, err := e.value.TryClose() - if !ok { - e.setActive(true) - toClose[idx] = nil - continue - } else { - e.setClosed() - } + closed, err := e.value.TryClose() if err != nil { c.log.With("object_id", e.id).Warnf("GC: object close error: %v", err) } + if !closed { + e.setActive(true) + continue + } else { + closedNum++ + c.mu.Lock() + e.setClosed() + delete(c.data, e.id) + c.mu.Unlock() + } } - toClose = slice.DiscardFromSlice(toClose, func(e *entry) bool { - return e == nil - }) - c.log.Infof("GC: removed %d; cache size: %d", len(toClose), size) - if len(toClose) > 0 && c.metrics != nil { - c.metrics.gc.Add(float64(len(toClose))) - } - c.mu.Lock() - for _, e := range toClose { - delete(c.data, e.id) - } - c.mu.Unlock() + c.metricsClosed(closedNum, size) } func (c *oCache) Len() int { @@ -347,9 +318,28 @@ func (c *oCache) Close() (err error) { } c.mu.Unlock() for _, e := range toClose { - if _, err := c.remove(e, false); err != ErrNotExists { + if _, err := c.remove(e); err != ErrNotExists { c.log.With("object_id", e.id).Warnf("cache close: object close error: %v", err) } } return nil } + +func (c *oCache) metricsGet(hit bool) { + if c.metrics == nil { + return + } + if hit { + c.metrics.hit.Inc() + } else { + c.metrics.miss.Inc() + } +} + +func (c *oCache) metricsClosed(closedLen, size int) { + c.log.Infof("GC: removed %d; cache size: %d", closedLen, size) + if c.metrics == nil || closedLen == 0 { + return + } + c.metrics.gc.Add(float64(closedLen)) +} From ca2ea8cef99b3002a91dd08b498bff9102dd878e Mon Sep 17 00:00:00 2001 From: mcrakhman Date: Thu, 9 Mar 2023 22:03:45 +0100 Subject: [PATCH 06/12] Add some fuzzy tests --- app/ocache/ocache.go | 3 +- app/ocache/ocache_test.go | 254 +++++++++++++++++++++++++++++++++----- 2 files changed, 227 insertions(+), 30 deletions(-) diff --git a/app/ocache/ocache.go b/app/ocache/ocache.go index 931cb431..cf8f6f88 100644 --- a/app/ocache/ocache.go +++ b/app/ocache/ocache.go @@ -192,6 +192,7 @@ func (c *oCache) remove(e *entry) (ok bool, err error) { } _, curState := e.setClosing(true) if curState == entryStateClosing { + ok = true err = e.value.Close() c.mu.Lock() e.setClosed() @@ -318,7 +319,7 @@ func (c *oCache) Close() (err error) { } c.mu.Unlock() for _, e := range toClose { - if _, err := c.remove(e); err != ErrNotExists { + if _, err := c.remove(e); err != nil && err != ErrNotExists { c.log.With("object_id", e.id).Warnf("cache close: object close error: %v", err) } } diff --git a/app/ocache/ocache_test.go b/app/ocache/ocache_test.go index 43773184..5a99e5cf 100644 --- a/app/ocache/ocache_test.go +++ b/app/ocache/ocache_test.go @@ -31,6 +31,9 @@ func NewTestObject(name string, tryReturn bool, closeCh chan struct{}) *testObje } func (t *testObject) Close() (err error) { + if t.closeCalled || (t.tryCloseCalled && t.tryReturn) { + panic("close called twice") + } t.closeCalled = true if t.closeCh != nil { <-t.closeCh @@ -39,10 +42,13 @@ func (t *testObject) Close() (err error) { } func (t *testObject) TryClose() (res bool, err error) { + if t.closeCalled || (t.tryCloseCalled && t.tryReturn) { + panic("close called twice") + } t.tryCloseCalled = true if t.closeCh != nil { <-t.closeCh - return true, t.closeErr + return t.tryReturn, t.closeErr } return t.tryReturn, nil } @@ -263,36 +269,226 @@ func TestOCache_GC(t *testing.T) { } func Test_OCache_Remove(t *testing.T) { - closeCh := make(chan struct{}) - getCh := make(chan struct{}) - c := New(func(ctx context.Context, id string) (value Object, err error) { - return NewTestObject(id, false, closeCh), nil - }, WithTTL(time.Millisecond*10)) + t.Run("remove simple", func(t *testing.T) { + closeCh := make(chan struct{}) + getCh := make(chan struct{}) + c := New(func(ctx context.Context, id string) (value Object, err error) { + return NewTestObject(id, false, closeCh), nil + }, WithTTL(time.Millisecond*10)) - val, err := c.Get(context.TODO(), "id") - require.NoError(t, err) - require.NotNil(t, val) - assert.Equal(t, 1, c.Len()) - // removing the object, so we will wait on closing - go func() { - _, err := c.Remove("id") - require.NoError(t, err) - }() - time.Sleep(time.Millisecond * 40) - - var events []string - go func() { - _, err := c.Get(context.TODO(), "id") + val, err := c.Get(context.TODO(), "id") require.NoError(t, err) require.NotNil(t, val) - events = append(events, "get") - close(getCh) - }() - // sleeping to make sure that Get is called - time.Sleep(time.Millisecond * 40) - events = append(events, "close") - close(closeCh) + assert.Equal(t, 1, c.Len()) + // removing the object, so we will wait on closing + go func() { + _, err := c.Remove("id") + require.NoError(t, err) + }() + time.Sleep(time.Millisecond * 40) - <-getCh - require.Equal(t, []string{"close", "get"}, events) + var events []string + go func() { + _, err := c.Get(context.TODO(), "id") + require.NoError(t, err) + require.NotNil(t, val) + events = append(events, "get") + close(getCh) + }() + // sleeping to make sure that Get is called + time.Sleep(time.Millisecond * 40) + events = append(events, "close") + close(closeCh) + + <-getCh + require.Equal(t, []string{"close", "get"}, events) + }) + t.Run("test remove while gc, tryClose false", func(t *testing.T) { + closeCh := make(chan struct{}) + removeCh := make(chan struct{}) + + c := New(func(ctx context.Context, id string) (value Object, err error) { + return NewTestObject(id, false, closeCh), nil + }, WithTTL(time.Millisecond*10)) + val, err := c.Get(context.TODO(), "id") + require.NoError(t, err) + require.NotNil(t, val) + assert.Equal(t, 1, c.Len()) + time.Sleep(time.Millisecond * 40) + go c.GC() + time.Sleep(time.Millisecond * 40) + var events []string + go func() { + ok, err := c.Remove("id") + require.NoError(t, err) + require.True(t, ok) + events = append(events, "remove") + close(removeCh) + }() + time.Sleep(time.Millisecond * 40) + events = append(events, "close") + close(closeCh) + + <-removeCh + require.Equal(t, []string{"close", "remove"}, events) + }) + t.Run("test remove while gc, tryClose true", func(t *testing.T) { + closeCh := make(chan struct{}) + removeCh := make(chan struct{}) + + c := New(func(ctx context.Context, id string) (value Object, err error) { + return NewTestObject(id, true, closeCh), nil + }, WithTTL(time.Millisecond*10)) + val, err := c.Get(context.TODO(), "id") + require.NoError(t, err) + require.NotNil(t, val) + assert.Equal(t, 1, c.Len()) + time.Sleep(time.Millisecond * 40) + go c.GC() + time.Sleep(time.Millisecond * 40) + var events []string + go func() { + ok, err := c.Remove("id") + require.NoError(t, err) + require.False(t, ok) + events = append(events, "remove") + close(removeCh) + }() + time.Sleep(time.Millisecond * 40) + events = append(events, "close") + close(closeCh) + + <-removeCh + require.Equal(t, []string{"close", "remove"}, events) + }) + t.Run("test gc while remove, tryClose true", func(t *testing.T) { + closeCh := make(chan struct{}) + removeCh := make(chan struct{}) + + c := New(func(ctx context.Context, id string) (value Object, err error) { + return NewTestObject(id, true, closeCh), nil + }, WithTTL(time.Millisecond*10)) + val, err := c.Get(context.TODO(), "id") + require.NoError(t, err) + require.NotNil(t, val) + assert.Equal(t, 1, c.Len()) + go func() { + ok, err := c.Remove("id") + require.NoError(t, err) + require.True(t, ok) + close(removeCh) + }() + time.Sleep(20 * time.Millisecond) + c.GC() + close(closeCh) + <-removeCh + }) +} + +func TestOCacheFuzzy(t *testing.T) { + t.Run("test many objects gc, get and remove simultaneously, close after", func(t *testing.T) { + tryCloseIds := make(map[string]bool) + called := make(map[string]int) + max := 2000 + getId := func(i int) string { + return fmt.Sprintf("id%d", i) + } + for i := 0; i < max; i++ { + if i%2 == 1 { + tryCloseIds[getId(i)] = true + } else { + tryCloseIds[getId(i)] = false + } + } + c := New(func(ctx context.Context, id string) (value Object, err error) { + called[id] = called[id] + 1 + return NewTestObject(id, tryCloseIds[id], nil), nil + }, WithTTL(time.Nanosecond)) + + stopGC := make(chan struct{}) + wg := sync.WaitGroup{} + go func() { + for { + select { + case <-stopGC: + return + default: + c.GC() + } + } + }() + wg.Add(1) + go func() { + defer wg.Done() + for j := 0; j < 10; j++ { + for i := 0; i < max; i++ { + val, err := c.Get(context.TODO(), getId(i)) + require.NoError(t, err) + require.NotNil(t, val) + } + } + }() + wg.Add(1) + go func() { + defer wg.Done() + for j := 0; j < 10; j++ { + for i := 0; i < max; i++ { + c.Remove(getId(i)) + } + } + }() + wg.Wait() + close(stopGC) + err := c.Close() + require.NoError(t, err) + require.Equal(t, 0, c.Len()) + }) + t.Run("test many objects gc, get, remove and close simultaneously", func(t *testing.T) { + tryCloseIds := make(map[string]bool) + called := make(map[string]int) + max := 2000 + getId := func(i int) string { + return fmt.Sprintf("id%d", i) + } + for i := 0; i < max; i++ { + if i%2 == 1 { + tryCloseIds[getId(i)] = true + } else { + tryCloseIds[getId(i)] = false + } + } + c := New(func(ctx context.Context, id string) (value Object, err error) { + called[id] = called[id] + 1 + return NewTestObject(id, tryCloseIds[id], nil), nil + }, WithTTL(time.Nanosecond)) + + go func() { + for { + c.GC() + } + }() + go func() { + for j := 0; j < 10; j++ { + for i := 0; i < max; i++ { + val, err := c.Get(context.TODO(), getId(i)) + if err == ErrClosed { + return + } + require.NoError(t, err) + require.NotNil(t, val) + } + } + }() + go func() { + for j := 0; j < 10; j++ { + for i := 0; i < max; i++ { + c.Remove(getId(i)) + } + } + }() + time.Sleep(time.Millisecond) + err := c.Close() + require.NoError(t, err) + require.Equal(t, 0, c.Len()) + }) } From 43a35f48787d695938e825f3e17b61d14123a5b0 Mon Sep 17 00:00:00 2001 From: mcrakhman Date: Thu, 9 Mar 2023 22:52:31 +0100 Subject: [PATCH 07/12] Add tryclose everywhere --- commonspace/headsync/diffsyncer_test.go | 4 +++ commonspace/objectsync/msgpool.go | 7 ++++-- commonspace/objectsync/objectsync.go | 3 +-- commonspace/space.go | 33 ++++++++++++------------- commonspace/spaceservice.go | 2 ++ net/dialer/dialer.go | 2 +- net/peer/peer.go | 12 ++++++++- net/pool/pool_test.go | 4 +++ net/rpc/rpctest/pool.go | 4 +++ 9 files changed, 48 insertions(+), 23 deletions(-) diff --git a/commonspace/headsync/diffsyncer_test.go b/commonspace/headsync/diffsyncer_test.go index f9622ca6..2b163a19 100644 --- a/commonspace/headsync/diffsyncer_test.go +++ b/commonspace/headsync/diffsyncer_test.go @@ -51,6 +51,10 @@ func (p pushSpaceRequestMatcher) String() string { type mockPeer struct{} +func (m mockPeer) TryClose() (res bool, err error) { + return true, m.Close() +} + func (m mockPeer) Id() string { return "mockId" } diff --git a/commonspace/objectsync/msgpool.go b/commonspace/objectsync/msgpool.go index 3dd14a2e..533efc7e 100644 --- a/commonspace/objectsync/msgpool.go +++ b/commonspace/objectsync/msgpool.go @@ -3,7 +3,6 @@ package objectsync import ( "context" "fmt" - "github.com/anytypeio/any-sync/app/ocache" "github.com/anytypeio/any-sync/commonspace/objectsync/synchandler" "github.com/anytypeio/any-sync/commonspace/peermanager" "github.com/anytypeio/any-sync/commonspace/spacesyncproto" @@ -15,9 +14,13 @@ import ( "time" ) +type LastUsage interface { + LastUsage() time.Time +} + // MessagePool can be made generic to work with different streams type MessagePool interface { - ocache.ObjectLastUsage + LastUsage synchandler.SyncHandler peermanager.PeerManager SendSync(ctx context.Context, peerId string, message *spacesyncproto.ObjectSyncMessage) (reply *spacesyncproto.ObjectSyncMessage, err error) diff --git a/commonspace/objectsync/objectsync.go b/commonspace/objectsync/objectsync.go index 85c587d9..74b3f7fa 100644 --- a/commonspace/objectsync/objectsync.go +++ b/commonspace/objectsync/objectsync.go @@ -6,7 +6,6 @@ import ( "time" "github.com/anytypeio/any-sync/app/logger" - "github.com/anytypeio/any-sync/app/ocache" "github.com/anytypeio/any-sync/commonspace/object/syncobjectgetter" "github.com/anytypeio/any-sync/commonspace/objectsync/synchandler" "github.com/anytypeio/any-sync/commonspace/peermanager" @@ -20,7 +19,7 @@ import ( var log = logger.NewNamed("common.commonspace.objectsync") type ObjectSync interface { - ocache.ObjectLastUsage + LastUsage synchandler.SyncHandler MessagePool() MessagePool diff --git a/commonspace/space.go b/commonspace/space.go index d18c9e8a..00b38a8d 100644 --- a/commonspace/space.go +++ b/commonspace/space.go @@ -6,7 +6,6 @@ import ( "fmt" "github.com/anytypeio/any-sync/accountservice" "github.com/anytypeio/any-sync/app/logger" - "github.com/anytypeio/any-sync/app/ocache" "github.com/anytypeio/any-sync/commonspace/headsync" "github.com/anytypeio/any-sync/commonspace/object/acl/list" "github.com/anytypeio/any-sync/commonspace/object/acl/syncacl" @@ -81,9 +80,6 @@ func NewSpaceId(id string, repKey uint64) string { } type Space interface { - ocache.ObjectLocker - ocache.ObjectLastUsage - Id() string Init(ctx context.Context) error @@ -112,9 +108,10 @@ type Space interface { } type space struct { - id string - mu sync.RWMutex - header *spacesyncproto.RawSpaceHeaderWithId + id string + mu sync.RWMutex + header *spacesyncproto.RawSpaceHeaderWithId + spaceTTL time.Duration objectSync objectsync.ObjectSync headSync headsync.HeadSync @@ -134,16 +131,6 @@ type space struct { treesUsed *atomic.Int32 } -func (s *space) LastUsage() time.Time { - return s.objectSync.LastUsage() -} - -func (s *space) Locked() bool { - locked := s.treesUsed.Load() > 1 - log.With(zap.Int32("trees used", s.treesUsed.Load()), zap.Bool("locked", locked), zap.String("spaceId", s.id)).Debug("space lock status check") - return locked -} - func (s *space) Id() string { return s.id } @@ -462,3 +449,15 @@ func (s *space) Close() error { log.With(zap.String("id", s.id)).Debug("space closed") return mError.Err() } + +func (s *space) TryClose() (close bool, err error) { + if time.Now().Sub(s.objectSync.LastUsage()) < s.spaceTTL { + return false, nil + } + locked := s.treesUsed.Load() > 1 + log.With(zap.Int32("trees used", s.treesUsed.Load()), zap.Bool("locked", locked), zap.String("spaceId", s.id)).Debug("space lock status check") + if locked { + return false, nil + } + return true, s.Close() +} diff --git a/commonspace/spaceservice.go b/commonspace/spaceservice.go index 89e9283b..0e086923 100644 --- a/commonspace/spaceservice.go +++ b/commonspace/spaceservice.go @@ -19,6 +19,7 @@ import ( "github.com/anytypeio/any-sync/net/pool" "github.com/anytypeio/any-sync/nodeconf" "sync/atomic" + "time" ) const CName = "common.commonspace" @@ -162,6 +163,7 @@ func (s *spaceService) NewSpace(ctx context.Context, id string) (Space, error) { treesUsed: &atomic.Int32{}, isClosed: spaceIsClosed, isDeleted: spaceIsDeleted, + spaceTTL: time.Duration(s.config.GCTTL) * time.Second, } return sp, nil } diff --git a/net/dialer/dialer.go b/net/dialer/dialer.go index 7252a4f3..64b0c191 100644 --- a/net/dialer/dialer.go +++ b/net/dialer/dialer.go @@ -100,7 +100,7 @@ func (d *dialer) Dial(ctx context.Context, peerId string) (p peer.Peer, err erro if err != nil { return } - return peer.NewPeer(sc, conn), nil + return peer.NewPeer(sc, conn, time.Minute), nil } func (d *dialer) handshake(ctx context.Context, addr string) (conn drpc.Conn, sc sec.SecureConn, err error) { diff --git a/net/peer/peer.go b/net/peer/peer.go index 9c9f547d..bc9353d0 100644 --- a/net/peer/peer.go +++ b/net/peer/peer.go @@ -12,10 +12,11 @@ import ( var log = logger.NewNamed("peer") -func NewPeer(sc sec.SecureConn, conn drpc.Conn) Peer { +func NewPeer(sc sec.SecureConn, conn drpc.Conn, ttl time.Duration) Peer { return &peer{ id: sc.RemotePeer().String(), lastUsage: time.Now().Unix(), + ttl: ttl, sc: sc, Conn: conn, } @@ -25,11 +26,13 @@ type Peer interface { Id() string LastUsage() time.Time UpdateLastUsage() + TryClose() (res bool, err error) drpc.Conn } type peer struct { id string + ttl time.Duration lastUsage int64 sc sec.SecureConn drpc.Conn @@ -76,6 +79,13 @@ func (p *peer) UpdateLastUsage() { atomic.StoreInt64(&p.lastUsage, time.Now().Unix()) } +func (p *peer) TryClose() (res bool, err error) { + if time.Now().Sub(p.LastUsage()) < p.ttl { + return false, nil + } + return true, p.Close() +} + func (p *peer) Close() (err error) { log.Debug("peer close", zap.String("peerId", p.id)) return p.Conn.Close() diff --git a/net/pool/pool_test.go b/net/pool/pool_test.go index f913333c..8ae08e3b 100644 --- a/net/pool/pool_test.go +++ b/net/pool/pool_test.go @@ -194,6 +194,10 @@ func (t *testPeer) LastUsage() time.Time { func (t *testPeer) UpdateLastUsage() {} +func (t *testPeer) TryClose() (res bool, err error) { + return true, t.Close() +} + func (t *testPeer) Close() error { select { case <-t.closed: diff --git a/net/rpc/rpctest/pool.go b/net/rpc/rpctest/pool.go index 7fdbdda4..40b7bb1d 100644 --- a/net/rpc/rpctest/pool.go +++ b/net/rpc/rpctest/pool.go @@ -103,6 +103,10 @@ type testPeer struct { drpc.Conn } +func (t testPeer) TryClose() (res bool, err error) { + return true, t.Close() +} + func (t testPeer) Id() string { return t.id } From 83476a9e8488f823225f597db688d17729efb2e8 Mon Sep 17 00:00:00 2001 From: mcrakhman Date: Thu, 9 Mar 2023 22:56:59 +0100 Subject: [PATCH 08/12] Lower timeouts --- app/ocache/ocache_test.go | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/app/ocache/ocache_test.go b/app/ocache/ocache_test.go index 5a99e5cf..3a2941a1 100644 --- a/app/ocache/ocache_test.go +++ b/app/ocache/ocache_test.go @@ -148,7 +148,7 @@ func TestOCache_GC(t *testing.T) { assert.Equal(t, 1, c.Len()) c.GC() assert.Equal(t, 1, c.Len()) - time.Sleep(time.Millisecond * 30) + time.Sleep(time.Millisecond * 20) c.GC() assert.Equal(t, 0, c.Len()) }) @@ -164,11 +164,11 @@ func TestOCache_GC(t *testing.T) { require.NotNil(t, val) assert.Equal(t, 1, c.Len()) // making ttl pass - time.Sleep(time.Millisecond * 40) + time.Sleep(time.Millisecond * 20) // first gc will be run after 20 secs, so calling it manually go c.GC() // waiting until all objects are marked as closing - time.Sleep(time.Millisecond * 40) + time.Sleep(time.Millisecond * 20) var events []string go func() { _, err := c.Get(context.TODO(), "id") @@ -178,7 +178,7 @@ func TestOCache_GC(t *testing.T) { close(getCh) }() // sleeping to make sure that Get is called - time.Sleep(time.Millisecond * 40) + time.Sleep(time.Millisecond * 20) events = append(events, "close") close(closeCh) @@ -197,7 +197,7 @@ func TestOCache_GC(t *testing.T) { require.NoError(t, err) require.NotNil(t, val) assert.Equal(t, 1, c.Len()) - time.Sleep(time.Millisecond * 40) + time.Sleep(time.Millisecond * 20) begin := make(chan struct{}) wg := sync.WaitGroup{} once := sync.Once{} @@ -252,7 +252,7 @@ func TestOCache_GC(t *testing.T) { require.NotNil(t, val) } assert.Equal(t, max, c.Len()) - time.Sleep(time.Millisecond * 40) + time.Sleep(time.Millisecond * 20) c.GC() for i := 0; i < max; i++ { val, err := c.Get(context.TODO(), getId(i)) @@ -285,7 +285,7 @@ func Test_OCache_Remove(t *testing.T) { _, err := c.Remove("id") require.NoError(t, err) }() - time.Sleep(time.Millisecond * 40) + time.Sleep(time.Millisecond * 20) var events []string go func() { @@ -296,7 +296,7 @@ func Test_OCache_Remove(t *testing.T) { close(getCh) }() // sleeping to make sure that Get is called - time.Sleep(time.Millisecond * 40) + time.Sleep(time.Millisecond * 20) events = append(events, "close") close(closeCh) @@ -314,9 +314,9 @@ func Test_OCache_Remove(t *testing.T) { require.NoError(t, err) require.NotNil(t, val) assert.Equal(t, 1, c.Len()) - time.Sleep(time.Millisecond * 40) + time.Sleep(time.Millisecond * 20) go c.GC() - time.Sleep(time.Millisecond * 40) + time.Sleep(time.Millisecond * 20) var events []string go func() { ok, err := c.Remove("id") @@ -325,7 +325,7 @@ func Test_OCache_Remove(t *testing.T) { events = append(events, "remove") close(removeCh) }() - time.Sleep(time.Millisecond * 40) + time.Sleep(time.Millisecond * 20) events = append(events, "close") close(closeCh) @@ -343,9 +343,9 @@ func Test_OCache_Remove(t *testing.T) { require.NoError(t, err) require.NotNil(t, val) assert.Equal(t, 1, c.Len()) - time.Sleep(time.Millisecond * 40) + time.Sleep(time.Millisecond * 20) go c.GC() - time.Sleep(time.Millisecond * 40) + time.Sleep(time.Millisecond * 20) var events []string go func() { ok, err := c.Remove("id") @@ -354,7 +354,7 @@ func Test_OCache_Remove(t *testing.T) { events = append(events, "remove") close(removeCh) }() - time.Sleep(time.Millisecond * 40) + time.Sleep(time.Millisecond * 20) events = append(events, "close") close(closeCh) From 94f28430a378e593c10a3b4b1669fa1e20923190 Mon Sep 17 00:00:00 2001 From: mcrakhman Date: Thu, 9 Mar 2023 22:58:36 +0100 Subject: [PATCH 09/12] Remove called records in fuzzy test --- app/ocache/ocache_test.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/app/ocache/ocache_test.go b/app/ocache/ocache_test.go index 3a2941a1..9ed130df 100644 --- a/app/ocache/ocache_test.go +++ b/app/ocache/ocache_test.go @@ -388,7 +388,6 @@ func Test_OCache_Remove(t *testing.T) { func TestOCacheFuzzy(t *testing.T) { t.Run("test many objects gc, get and remove simultaneously, close after", func(t *testing.T) { tryCloseIds := make(map[string]bool) - called := make(map[string]int) max := 2000 getId := func(i int) string { return fmt.Sprintf("id%d", i) @@ -401,7 +400,6 @@ func TestOCacheFuzzy(t *testing.T) { } } c := New(func(ctx context.Context, id string) (value Object, err error) { - called[id] = called[id] + 1 return NewTestObject(id, tryCloseIds[id], nil), nil }, WithTTL(time.Nanosecond)) @@ -445,7 +443,6 @@ func TestOCacheFuzzy(t *testing.T) { }) t.Run("test many objects gc, get, remove and close simultaneously", func(t *testing.T) { tryCloseIds := make(map[string]bool) - called := make(map[string]int) max := 2000 getId := func(i int) string { return fmt.Sprintf("id%d", i) @@ -458,7 +455,6 @@ func TestOCacheFuzzy(t *testing.T) { } } c := New(func(ctx context.Context, id string) (value Object, err error) { - called[id] = called[id] + 1 return NewTestObject(id, tryCloseIds[id], nil), nil }, WithTTL(time.Nanosecond)) From 0c797412166d0080f8bd675d560ab1a8f7cadfe0 Mon Sep 17 00:00:00 2001 From: mcrakhman Date: Thu, 9 Mar 2023 23:09:26 +0100 Subject: [PATCH 10/12] Update objecttree with tryclose --- .../objecttree/mock_objecttree/mock_objecttree.go | 15 +++++++++++++++ commonspace/object/tree/objecttree/objecttree.go | 5 +++++ .../tree/synctree/mock_synctree/mock_synctree.go | 15 +++++++++++++++ commonspace/object/tree/synctree/synctree.go | 4 ++++ .../mock_coordinatorclient.go | 11 ++++++----- 5 files changed, 45 insertions(+), 5 deletions(-) diff --git a/commonspace/object/tree/objecttree/mock_objecttree/mock_objecttree.go b/commonspace/object/tree/objecttree/mock_objecttree/mock_objecttree.go index 541fa779..d64e7dea 100644 --- a/commonspace/object/tree/objecttree/mock_objecttree/mock_objecttree.go +++ b/commonspace/object/tree/objecttree/mock_objecttree/mock_objecttree.go @@ -336,6 +336,21 @@ func (mr *MockObjectTreeMockRecorder) Storage() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Storage", reflect.TypeOf((*MockObjectTree)(nil).Storage)) } +// TryClose mocks base method. +func (m *MockObjectTree) TryClose() (bool, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "TryClose") + ret0, _ := ret[0].(bool) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// TryClose indicates an expected call of TryClose. +func (mr *MockObjectTreeMockRecorder) TryClose() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "TryClose", reflect.TypeOf((*MockObjectTree)(nil).TryClose)) +} + // TryLock mocks base method. func (m *MockObjectTree) TryLock() bool { m.ctrl.T.Helper() diff --git a/commonspace/object/tree/objecttree/objecttree.go b/commonspace/object/tree/objecttree/objecttree.go index 31abc01a..003cdd39 100644 --- a/commonspace/object/tree/objecttree/objecttree.go +++ b/commonspace/object/tree/objecttree/objecttree.go @@ -81,6 +81,7 @@ type ObjectTree interface { Delete() error Close() error + TryClose() (bool, error) } type objectTree struct { @@ -555,6 +556,10 @@ func (ot *objectTree) Root() *Change { return ot.tree.Root() } +func (ot *objectTree) TryClose() (bool, error) { + return true, ot.Close() +} + func (ot *objectTree) Close() error { return nil } diff --git a/commonspace/object/tree/synctree/mock_synctree/mock_synctree.go b/commonspace/object/tree/synctree/mock_synctree/mock_synctree.go index d7fe181f..31ee3607 100644 --- a/commonspace/object/tree/synctree/mock_synctree/mock_synctree.go +++ b/commonspace/object/tree/synctree/mock_synctree/mock_synctree.go @@ -487,6 +487,21 @@ func (mr *MockSyncTreeMockRecorder) SyncWithPeer(arg0, arg1 interface{}) *gomock return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SyncWithPeer", reflect.TypeOf((*MockSyncTree)(nil).SyncWithPeer), arg0, arg1) } +// TryClose mocks base method. +func (m *MockSyncTree) TryClose() (bool, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "TryClose") + ret0, _ := ret[0].(bool) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// TryClose indicates an expected call of TryClose. +func (mr *MockSyncTreeMockRecorder) TryClose() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "TryClose", reflect.TypeOf((*MockSyncTree)(nil).TryClose)) +} + // TryLock mocks base method. func (m *MockSyncTree) TryLock() bool { m.ctrl.T.Helper() diff --git a/commonspace/object/tree/synctree/synctree.go b/commonspace/object/tree/synctree/synctree.go index f25e14d0..fe429768 100644 --- a/commonspace/object/tree/synctree/synctree.go +++ b/commonspace/object/tree/synctree/synctree.go @@ -209,6 +209,10 @@ func (s *syncTree) Delete() (err error) { return } +func (s *syncTree) TryClose() (bool, error) { + return true, s.Close() +} + func (s *syncTree) Close() (err error) { log.Debug("closing sync tree", zap.String("id", s.Id())) defer func() { diff --git a/coordinator/coordinatorclient/mock_coordinatorclient/mock_coordinatorclient.go b/coordinator/coordinatorclient/mock_coordinatorclient/mock_coordinatorclient.go index b6b6931e..7216e1eb 100644 --- a/coordinator/coordinatorclient/mock_coordinatorclient/mock_coordinatorclient.go +++ b/coordinator/coordinatorclient/mock_coordinatorclient/mock_coordinatorclient.go @@ -38,11 +38,12 @@ func (m *MockCoordinatorClient) EXPECT() *MockCoordinatorClientMockRecorder { } // ChangeStatus mocks base method. -func (m *MockCoordinatorClient) ChangeStatus(arg0 context.Context, arg1 string, arg2 *treechangeproto.RawTreeChangeWithId) error { +func (m *MockCoordinatorClient) ChangeStatus(arg0 context.Context, arg1 string, arg2 *treechangeproto.RawTreeChangeWithId) (*coordinatorproto.SpaceStatusPayload, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "ChangeStatus", arg0, arg1, arg2) - ret0, _ := ret[0].(error) - return ret0 + ret0, _ := ret[0].(*coordinatorproto.SpaceStatusPayload) + ret1, _ := ret[1].(error) + return ret0, ret1 } // ChangeStatus indicates an expected call of ChangeStatus. @@ -110,10 +111,10 @@ func (mr *MockCoordinatorClientMockRecorder) SpaceSign(arg0, arg1, arg2 interfac } // StatusCheck mocks base method. -func (m *MockCoordinatorClient) StatusCheck(arg0 context.Context, arg1 string) (*coordinatorproto.SpaceStatusCheckResponse, error) { +func (m *MockCoordinatorClient) StatusCheck(arg0 context.Context, arg1 string) (*coordinatorproto.SpaceStatusPayload, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "StatusCheck", arg0, arg1) - ret0, _ := ret[0].(*coordinatorproto.SpaceStatusCheckResponse) + ret0, _ := ret[0].(*coordinatorproto.SpaceStatusPayload) ret1, _ := ret[1].(error) return ret0, ret1 } From 067ad8cf16e5f8568aeef3d6e9ebef438f479775 Mon Sep 17 00:00:00 2001 From: mcrakhman Date: Thu, 9 Mar 2023 23:11:20 +0100 Subject: [PATCH 11/12] Add TryClose to space interface --- commonspace/space.go | 1 + 1 file changed, 1 insertion(+) diff --git a/commonspace/space.go b/commonspace/space.go index 00b38a8d..fafee759 100644 --- a/commonspace/space.go +++ b/commonspace/space.go @@ -104,6 +104,7 @@ type Space interface { HandleMessage(ctx context.Context, msg HandleMessage) (err error) + TryClose() (close bool, err error) Close() error } From 74f39239c6f82204fb7345eb65b035d9fca4d4a9 Mon Sep 17 00:00:00 2001 From: mcrakhman Date: Fri, 10 Mar 2023 10:17:57 +0100 Subject: [PATCH 12/12] Update tryclose with ttl --- app/ocache/ocache.go | 5 +++-- app/ocache/ocache_test.go | 2 +- commonspace/headsync/diffsyncer_test.go | 2 +- .../objecttree/mock_objecttree/mock_objecttree.go | 9 +++++---- commonspace/object/tree/objecttree/objecttree.go | 5 +++-- .../tree/synctree/mock_synctree/mock_synctree.go | 9 +++++---- commonspace/object/tree/synctree/synctree.go | 3 ++- commonspace/space.go | 13 ++++++------- commonspace/spaceservice.go | 2 -- net/dialer/dialer.go | 2 +- net/peer/peer.go | 9 ++++----- net/pool/pool_test.go | 2 +- net/rpc/rpctest/pool.go | 2 +- 13 files changed, 33 insertions(+), 32 deletions(-) diff --git a/app/ocache/ocache.go b/app/ocache/ocache.go index cf8f6f88..7375c643 100644 --- a/app/ocache/ocache.go +++ b/app/ocache/ocache.go @@ -67,7 +67,7 @@ func New(loadFunc LoadFunc, opts ...Option) OCache { type Object interface { Close() (err error) - TryClose() (res bool, err error) + TryClose(objectTTL time.Duration) (res bool, err error) } type OCache interface { @@ -127,6 +127,7 @@ Load: load = true c.data[id] = e } + e.lastUsage = time.Now() c.mu.Unlock() reload, err := e.waitClose(ctx, id) if err != nil { @@ -281,7 +282,7 @@ func (c *oCache) GC() { if prevState == entryStateClosing || prevState == entryStateClosed { continue } - closed, err := e.value.TryClose() + closed, err := e.value.TryClose(c.ttl) if err != nil { c.log.With("object_id", e.id).Warnf("GC: object close error: %v", err) } diff --git a/app/ocache/ocache_test.go b/app/ocache/ocache_test.go index 9ed130df..14a59b1a 100644 --- a/app/ocache/ocache_test.go +++ b/app/ocache/ocache_test.go @@ -41,7 +41,7 @@ func (t *testObject) Close() (err error) { return t.closeErr } -func (t *testObject) TryClose() (res bool, err error) { +func (t *testObject) TryClose(objectTTL time.Duration) (res bool, err error) { if t.closeCalled || (t.tryCloseCalled && t.tryReturn) { panic("close called twice") } diff --git a/commonspace/headsync/diffsyncer_test.go b/commonspace/headsync/diffsyncer_test.go index 2b163a19..ae40e5a7 100644 --- a/commonspace/headsync/diffsyncer_test.go +++ b/commonspace/headsync/diffsyncer_test.go @@ -51,7 +51,7 @@ func (p pushSpaceRequestMatcher) String() string { type mockPeer struct{} -func (m mockPeer) TryClose() (res bool, err error) { +func (m mockPeer) TryClose(objectTTL time.Duration) (res bool, err error) { return true, m.Close() } diff --git a/commonspace/object/tree/objecttree/mock_objecttree/mock_objecttree.go b/commonspace/object/tree/objecttree/mock_objecttree/mock_objecttree.go index d64e7dea..4d5898fe 100644 --- a/commonspace/object/tree/objecttree/mock_objecttree/mock_objecttree.go +++ b/commonspace/object/tree/objecttree/mock_objecttree/mock_objecttree.go @@ -7,6 +7,7 @@ package mock_objecttree import ( context "context" reflect "reflect" + time "time" list "github.com/anytypeio/any-sync/commonspace/object/acl/list" objecttree "github.com/anytypeio/any-sync/commonspace/object/tree/objecttree" @@ -337,18 +338,18 @@ func (mr *MockObjectTreeMockRecorder) Storage() *gomock.Call { } // TryClose mocks base method. -func (m *MockObjectTree) TryClose() (bool, error) { +func (m *MockObjectTree) TryClose(arg0 time.Duration) (bool, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "TryClose") + ret := m.ctrl.Call(m, "TryClose", arg0) ret0, _ := ret[0].(bool) ret1, _ := ret[1].(error) return ret0, ret1 } // TryClose indicates an expected call of TryClose. -func (mr *MockObjectTreeMockRecorder) TryClose() *gomock.Call { +func (mr *MockObjectTreeMockRecorder) TryClose(arg0 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "TryClose", reflect.TypeOf((*MockObjectTree)(nil).TryClose)) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "TryClose", reflect.TypeOf((*MockObjectTree)(nil).TryClose), arg0) } // TryLock mocks base method. diff --git a/commonspace/object/tree/objecttree/objecttree.go b/commonspace/object/tree/objecttree/objecttree.go index 003cdd39..f72a5a5d 100644 --- a/commonspace/object/tree/objecttree/objecttree.go +++ b/commonspace/object/tree/objecttree/objecttree.go @@ -5,6 +5,7 @@ import ( "context" "errors" "sync" + "time" "github.com/anytypeio/any-sync/commonspace/object/acl/aclrecordproto" "github.com/anytypeio/any-sync/commonspace/object/acl/list" @@ -81,7 +82,7 @@ type ObjectTree interface { Delete() error Close() error - TryClose() (bool, error) + TryClose(objectTTL time.Duration) (bool, error) } type objectTree struct { @@ -556,7 +557,7 @@ func (ot *objectTree) Root() *Change { return ot.tree.Root() } -func (ot *objectTree) TryClose() (bool, error) { +func (ot *objectTree) TryClose(objectTTL time.Duration) (bool, error) { return true, ot.Close() } diff --git a/commonspace/object/tree/synctree/mock_synctree/mock_synctree.go b/commonspace/object/tree/synctree/mock_synctree/mock_synctree.go index 31ee3607..a4205132 100644 --- a/commonspace/object/tree/synctree/mock_synctree/mock_synctree.go +++ b/commonspace/object/tree/synctree/mock_synctree/mock_synctree.go @@ -7,6 +7,7 @@ package mock_synctree import ( context "context" reflect "reflect" + time "time" list "github.com/anytypeio/any-sync/commonspace/object/acl/list" objecttree "github.com/anytypeio/any-sync/commonspace/object/tree/objecttree" @@ -488,18 +489,18 @@ func (mr *MockSyncTreeMockRecorder) SyncWithPeer(arg0, arg1 interface{}) *gomock } // TryClose mocks base method. -func (m *MockSyncTree) TryClose() (bool, error) { +func (m *MockSyncTree) TryClose(arg0 time.Duration) (bool, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "TryClose") + ret := m.ctrl.Call(m, "TryClose", arg0) ret0, _ := ret[0].(bool) ret1, _ := ret[1].(error) return ret0, ret1 } // TryClose indicates an expected call of TryClose. -func (mr *MockSyncTreeMockRecorder) TryClose() *gomock.Call { +func (mr *MockSyncTreeMockRecorder) TryClose(arg0 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "TryClose", reflect.TypeOf((*MockSyncTree)(nil).TryClose)) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "TryClose", reflect.TypeOf((*MockSyncTree)(nil).TryClose), arg0) } // TryLock mocks base method. diff --git a/commonspace/object/tree/synctree/synctree.go b/commonspace/object/tree/synctree/synctree.go index fe429768..56d07c1c 100644 --- a/commonspace/object/tree/synctree/synctree.go +++ b/commonspace/object/tree/synctree/synctree.go @@ -3,6 +3,7 @@ package synctree import ( "context" "errors" + "time" "github.com/anytypeio/any-sync/app/logger" "github.com/anytypeio/any-sync/commonspace/object/acl/list" @@ -209,7 +210,7 @@ func (s *syncTree) Delete() (err error) { return } -func (s *syncTree) TryClose() (bool, error) { +func (s *syncTree) TryClose(objectTTL time.Duration) (bool, error) { return true, s.Close() } diff --git a/commonspace/space.go b/commonspace/space.go index fafee759..6f2a3af2 100644 --- a/commonspace/space.go +++ b/commonspace/space.go @@ -104,15 +104,14 @@ type Space interface { HandleMessage(ctx context.Context, msg HandleMessage) (err error) - TryClose() (close bool, err error) + TryClose(objectTTL time.Duration) (close bool, err error) Close() error } type space struct { - id string - mu sync.RWMutex - header *spacesyncproto.RawSpaceHeaderWithId - spaceTTL time.Duration + id string + mu sync.RWMutex + header *spacesyncproto.RawSpaceHeaderWithId objectSync objectsync.ObjectSync headSync headsync.HeadSync @@ -451,8 +450,8 @@ func (s *space) Close() error { return mError.Err() } -func (s *space) TryClose() (close bool, err error) { - if time.Now().Sub(s.objectSync.LastUsage()) < s.spaceTTL { +func (s *space) TryClose(objectTTL time.Duration) (close bool, err error) { + if time.Now().Sub(s.objectSync.LastUsage()) < objectTTL { return false, nil } locked := s.treesUsed.Load() > 1 diff --git a/commonspace/spaceservice.go b/commonspace/spaceservice.go index 0e086923..89e9283b 100644 --- a/commonspace/spaceservice.go +++ b/commonspace/spaceservice.go @@ -19,7 +19,6 @@ import ( "github.com/anytypeio/any-sync/net/pool" "github.com/anytypeio/any-sync/nodeconf" "sync/atomic" - "time" ) const CName = "common.commonspace" @@ -163,7 +162,6 @@ func (s *spaceService) NewSpace(ctx context.Context, id string) (Space, error) { treesUsed: &atomic.Int32{}, isClosed: spaceIsClosed, isDeleted: spaceIsDeleted, - spaceTTL: time.Duration(s.config.GCTTL) * time.Second, } return sp, nil } diff --git a/net/dialer/dialer.go b/net/dialer/dialer.go index 64b0c191..7252a4f3 100644 --- a/net/dialer/dialer.go +++ b/net/dialer/dialer.go @@ -100,7 +100,7 @@ func (d *dialer) Dial(ctx context.Context, peerId string) (p peer.Peer, err erro if err != nil { return } - return peer.NewPeer(sc, conn, time.Minute), nil + return peer.NewPeer(sc, conn), nil } func (d *dialer) handshake(ctx context.Context, addr string) (conn drpc.Conn, sc sec.SecureConn, err error) { diff --git a/net/peer/peer.go b/net/peer/peer.go index bc9353d0..c96cd4ba 100644 --- a/net/peer/peer.go +++ b/net/peer/peer.go @@ -12,11 +12,10 @@ import ( var log = logger.NewNamed("peer") -func NewPeer(sc sec.SecureConn, conn drpc.Conn, ttl time.Duration) Peer { +func NewPeer(sc sec.SecureConn, conn drpc.Conn) Peer { return &peer{ id: sc.RemotePeer().String(), lastUsage: time.Now().Unix(), - ttl: ttl, sc: sc, Conn: conn, } @@ -26,7 +25,7 @@ type Peer interface { Id() string LastUsage() time.Time UpdateLastUsage() - TryClose() (res bool, err error) + TryClose(objectTTL time.Duration) (res bool, err error) drpc.Conn } @@ -79,8 +78,8 @@ func (p *peer) UpdateLastUsage() { atomic.StoreInt64(&p.lastUsage, time.Now().Unix()) } -func (p *peer) TryClose() (res bool, err error) { - if time.Now().Sub(p.LastUsage()) < p.ttl { +func (p *peer) TryClose(objectTTL time.Duration) (res bool, err error) { + if time.Now().Sub(p.LastUsage()) < objectTTL { return false, nil } return true, p.Close() diff --git a/net/pool/pool_test.go b/net/pool/pool_test.go index 8ae08e3b..ce3876e0 100644 --- a/net/pool/pool_test.go +++ b/net/pool/pool_test.go @@ -194,7 +194,7 @@ func (t *testPeer) LastUsage() time.Time { func (t *testPeer) UpdateLastUsage() {} -func (t *testPeer) TryClose() (res bool, err error) { +func (t *testPeer) TryClose(objectTTL time.Duration) (res bool, err error) { return true, t.Close() } diff --git a/net/rpc/rpctest/pool.go b/net/rpc/rpctest/pool.go index 40b7bb1d..630cbb6a 100644 --- a/net/rpc/rpctest/pool.go +++ b/net/rpc/rpctest/pool.go @@ -103,7 +103,7 @@ type testPeer struct { drpc.Conn } -func (t testPeer) TryClose() (res bool, err error) { +func (t testPeer) TryClose(objectTTL time.Duration) (res bool, err error) { return true, t.Close() }