From f8ae4857940e005ec3360bb9c3fb9eb4283cdbce Mon Sep 17 00:00:00 2001 From: norwnd Date: Fri, 10 Mar 2023 11:16:58 +0300 Subject: [PATCH 1/2] client/core: clarify candle cache assumptions --- client/core/bookie.go | 28 +++++++++++++++++++++------- 1 file changed, 21 insertions(+), 7 deletions(-) diff --git a/client/core/bookie.go b/client/core/bookie.go index 7f9dea9750..4b43932cb6 100644 --- a/client/core/bookie.go +++ b/client/core/bookie.go @@ -62,13 +62,21 @@ func (f *bookFeed) Candles(durStr string) error { } // candleCache adds synchronization and an on/off switch to *candles.Cache. +// NOTE: Occasionally we are missing candle updates (epoch_report notification) +// when downloading/applying server snapshot. For candle cache such atomicity property +// isn't supported at the moment, that's both neither on client nor on server side +// (so it's 2 different places to adjust, if atomicity is desirable). It isn't as +// noticeable for candles as it is for orders (where lack of similar atomicity property +// would result in ghost orders), still without this candle chart doesn't reflect what +// actually happened on dex server with 100% accuracy in real time (when cache resets, +// e.g. on dexc restart, this discrepancy is resolved). type candleCache struct { *candles.Cache // candleMtx protects the integrity of candles.Cache (e.g. we can't update // it while making copy at the same time), so it represents a consistent // data snapshot. candleMtx sync.RWMutex - on uint32 + on uint32 // whether cache has been initialized } // init resets the candles with the supplied set. @@ -81,6 +89,14 @@ func (c *candleCache) init(in []*msgjson.Candle) { } } +// snapshot takes snapshot of candle cache (preventing concurrent cache updates). +func (c *candleCache) snapshot() []msgjson.Candle { + c.candleMtx.RLock() + defer c.candleMtx.RUnlock() + + return c.CandlesCopy() +} + // addCandle adds the candle using candles.Cache.Add. It returns most recent candle // in cache. func (c *candleCache) addCandle(msgCandle *msgjson.Candle) (recent msgjson.Candle, ok bool) { @@ -100,7 +116,8 @@ func (c *candleCache) addCandle(msgCandle *msgjson.Candle) (recent msgjson.Candl // supplied close() callback. type bookie struct { *orderbook.OrderBook - dc *dexConnection + dc *dexConnection + // candleCaches is indexing candle caches by durations [5m,1h,24h]. candleCaches map[string]*candleCache log dex.Logger @@ -178,7 +195,7 @@ func (b *bookie) logEpochReport(note *msgjson.EpochReportNote) error { if err != nil { return err } - if note.Candle.EndStamp == 0 { + if note.Candle.EndStamp == 0 { // should never happen return fmt.Errorf("epoch report has zero-valued candle end stamp") } @@ -272,9 +289,6 @@ func (b *bookie) candles(durStr string, feedID uint32) error { return } dur, _ := time.ParseDuration(durStr) - cache.candleMtx.RLock() - cdls := cache.CandlesCopy() - cache.candleMtx.RUnlock() f.c <- &BookUpdate{ Action: FreshCandlesAction, Host: b.dc.acct.host, @@ -282,7 +296,7 @@ func (b *bookie) candles(durStr string, feedID uint32) error { Payload: &CandlesPayload{ Dur: durStr, DurMilliSecs: uint64(dur.Milliseconds()), - Candles: cdls, + Candles: cache.snapshot(), }, } }() From a77fe80daf9310e7d718369e00d66b5dfd1743ef Mon Sep 17 00:00:00 2001 From: norwnd Date: Sat, 11 Mar 2023 08:49:44 +0300 Subject: [PATCH 2/2] Minor clarification --- client/core/bookie.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/client/core/bookie.go b/client/core/bookie.go index 4b43932cb6..710d3751a5 100644 --- a/client/core/bookie.go +++ b/client/core/bookie.go @@ -69,7 +69,8 @@ func (f *bookFeed) Candles(durStr string) error { // noticeable for candles as it is for orders (where lack of similar atomicity property // would result in ghost orders), still without this candle chart doesn't reflect what // actually happened on dex server with 100% accuracy in real time (when cache resets, -// e.g. on dexc restart, this discrepancy is resolved). +// e.g. on dexc restart, this discrepancy is resolved for older candles, it affects +// only recent ones). type candleCache struct { *candles.Cache // candleMtx protects the integrity of candles.Cache (e.g. we can't update @@ -300,6 +301,10 @@ func (b *bookie) candles(durStr string, feedID uint32) error { }, } }() + // Two or more concurrent calls might fall through here, resulting in cache + // being initialized with cache.init multiple times. It's OK though because + // candleCache is not 100% consistent with server's anyway, see comments on + // candleCache struct for details, and probably doesn't need to be. if atomic.LoadUint32(&cache.on) == 1 { return nil }