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.

Sign up for GitHub

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

engine/sync-manager: add config support #1326

Merged
merged 13 commits into from
Aug 29, 2023
Merged

engine/sync-manager: add config support #1326

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
0bbbe6f
allows sync manager customisation for values and logs
gloriousCode Aug 22, 2023
9a04818
config-example add
gloriousCode Aug 22, 2023
e46f1f9
who doesnt like more coverage?
gloriousCode Aug 22, 2023
46c69f5
ensures you can actually disable it via config el oh el
gloriousCode Aug 22, 2023
a2ed2cc
less ifs, better control
gloriousCode Aug 22, 2023
ccb1bc0
fix verbose
gloriousCode Aug 22, 2023
2e485fd
sync trades default false
gloriousCode Aug 22, 2023
56242f8
fix summary being printed when not enabled
gloriousCode Aug 23, 2023
c373c59
fixes config checker and output
gloriousCode Aug 23, 2023
f706f15
nits
gloriousCode Aug 24, 2023
27346d5
I can put this behind me now
gloriousCode Aug 24, 2023
8a82a77
Fixed logCaSiNg
gloriousCode Aug 29, 2023
d2d274d
combines if statements
gloriousCode Aug 29, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 54 additions & 0 deletions config/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -696,6 +696,59 @@ func (c *Config) GetAvailablePairs(exchName string, assetType asset.Item) (curre
return pairs.Format(pairFormat), nil
}

// GetDefaultSyncManagerConfig returns a config with default values
func GetDefaultSyncManagerConfig() SyncManagerConfig {
return SyncManagerConfig{
Enabled: true,
SynchronizeTicker: true,
SynchronizeOrderbook: true,
SynchronizeTrades: false,
SynchronizeContinuously: true,
TimeoutREST: DefaultSyncerTimeoutREST,
TimeoutWebsocket: DefaultSyncerTimeoutWebsocket,
NumWorkers: DefaultSyncerWorkers,
FiatDisplayCurrency: currency.USD,
PairFormatDisplay: &currency.PairFormat{
Delimiter: "-",
Uppercase: true,
},
Verbose: false,
LogSyncUpdateEvents: true,
LogSwitchProtocolEvents: true,
LogInitialSyncEvents: true,
}
}

// CheckSyncManagerConfig checks config for valid values
// sets defaults if values are invalid
func (c *Config) CheckSyncManagerConfig() {
m.Lock()
defer m.Unlock()
if c.SyncManagerConfig == (SyncManagerConfig{}) {
c.SyncManagerConfig = GetDefaultSyncManagerConfig()
return
}
if c.SyncManagerConfig.TimeoutWebsocket <= 0 {
shazbert marked this conversation as resolved.
Show resolved Hide resolved
log.Warnf(log.ConfigMgr, "invalid sync manager websocket timeout value %v, defaulting to %v\n", c.SyncManagerConfig.TimeoutWebsocket, DefaultSyncerTimeoutWebsocket)
gloriousCode marked this conversation as resolved.
Show resolved Hide resolved
c.SyncManagerConfig.TimeoutWebsocket = DefaultSyncerTimeoutWebsocket
}
if c.SyncManagerConfig.PairFormatDisplay == nil {
log.Warnf(log.ConfigMgr, "invalid sync manager pair format value %v, using default format eg BTC-USD\n", c.SyncManagerConfig.PairFormatDisplay)
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

gloriousCode marked this conversation as resolved.
Show resolved Hide resolved
c.SyncManagerConfig.PairFormatDisplay = &currency.PairFormat{
Uppercase: true,
Delimiter: currency.DashDelimiter,
}
}
if c.SyncManagerConfig.TimeoutREST <= 0 {
Copy link
Collaborator

Choose a reason for hiding this comment

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

if I purge fiat display currency it purges and never comes back and stops the sync manager from working🙏

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Well don't do that 😆 Will look into it

Copy link
Collaborator

Choose a reason for hiding this comment

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

This is still an issue, when I remove this fiatdisplayCurrency from the config it stops the starting of the syncMnager

log.Warnf(log.ConfigMgr, "invalid sync manager REST timeout value %v, defaulting to %v\n", c.SyncManagerConfig.TimeoutREST, DefaultSyncerTimeoutREST)
gloriousCode marked this conversation as resolved.
Show resolved Hide resolved
c.SyncManagerConfig.TimeoutREST = DefaultSyncerTimeoutREST
}
if c.SyncManagerConfig.NumWorkers <= 0 {
log.Warnf(log.ConfigMgr, "invalid sync manager worker count value %v, defaulting to %v\n", c.SyncManagerConfig.NumWorkers, DefaultSyncerWorkers)
gloriousCode marked this conversation as resolved.
Show resolved Hide resolved
c.SyncManagerConfig.NumWorkers = DefaultSyncerWorkers
}
}

// GetEnabledPairs returns a list of currency pairs for a specific exchange
func (c *Config) GetEnabledPairs(exchName string, assetType asset.Item) (currency.Pairs, error) {
exchCfg, err := c.GetExchangeConfig(exchName)
Expand Down Expand Up @@ -1730,6 +1783,7 @@ func (c *Config) CheckConfig() error {
c.CheckClientBankAccounts()
c.CheckBankAccountConfig()
c.CheckRemoteControlConfig()
c.CheckSyncManagerConfig()

err = c.CheckCurrencyConfigValues()
if err != nil {
Expand Down
43 changes: 43 additions & 0 deletions config/config_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2299,3 +2299,46 @@ func TestExchangeConfigValidate(t *testing.T) {
t.Fatalf("received: '%v' but expected: '%v'", err, nil)
}
}

func TestGetDefaultSyncManagerConfig(t *testing.T) {
t.Parallel()
cfg := GetDefaultSyncManagerConfig()
if cfg == (SyncManagerConfig{}) {
t.Error("expected config")
}
if cfg.TimeoutREST != DefaultSyncerTimeoutREST {
t.Errorf("expected %v, received %v", DefaultSyncerTimeoutREST, cfg.TimeoutREST)
}
}

func TestCheckSyncManagerConfig(t *testing.T) {
t.Parallel()
c := Config{}
if c.SyncManagerConfig != (SyncManagerConfig{}) {
t.Error("expected empty config")
}
c.CheckSyncManagerConfig()
if c.SyncManagerConfig.TimeoutREST != DefaultSyncerTimeoutREST {
t.Error("expected default config")
}
c.SyncManagerConfig.TimeoutWebsocket = -1
c.SyncManagerConfig.PairFormatDisplay = nil
c.SyncManagerConfig.TimeoutREST = -1
c.SyncManagerConfig.NumWorkers = -1
c.CurrencyPairFormat = &currency.PairFormat{
Uppercase: true,
}
c.CheckSyncManagerConfig()
if c.SyncManagerConfig.TimeoutWebsocket != DefaultSyncerTimeoutWebsocket {
t.Errorf("received %v expected %v", c.SyncManagerConfig.TimeoutWebsocket, DefaultSyncerTimeoutWebsocket)
}
if c.SyncManagerConfig.PairFormatDisplay == nil {
t.Errorf("received %v expected %v", c.SyncManagerConfig.PairFormatDisplay, c.CurrencyPairFormat)
}
if c.SyncManagerConfig.TimeoutREST != DefaultSyncerTimeoutREST {
t.Errorf("received %v expected %v", c.SyncManagerConfig.TimeoutREST, DefaultSyncerTimeoutREST)
}
if c.SyncManagerConfig.NumWorkers != DefaultSyncerWorkers {
t.Errorf("received %v expected %v", c.SyncManagerConfig.NumWorkers, DefaultSyncerWorkers)
}
}
26 changes: 26 additions & 0 deletions config/config_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,12 @@ const (
defaultCurrencyStateManagerDelay = time.Minute
defaultMaxJobsPerCycle = 5
DefaultOrderbookPublishPeriod = time.Second * 10
// DefaultSyncerWorkers limits the number of sync workers
DefaultSyncerWorkers = 15
// DefaultSyncerTimeoutREST the default time to switch from REST to websocket protocols without a response
DefaultSyncerTimeoutREST = time.Second * 15
// DefaultSyncerTimeoutWebsocket the default time to switch from websocket to REST protocols without a response
DefaultSyncerTimeoutWebsocket = time.Minute
)

// Constants here hold some messages
Expand Down Expand Up @@ -81,6 +87,7 @@ type Config struct {
GlobalHTTPTimeout time.Duration `json:"globalHTTPTimeout"`
Database database.Config `json:"database"`
Logging log.Config `json:"logging"`
SyncManagerConfig SyncManagerConfig `json:"syncManager"`
ConnectionMonitor ConnectionMonitorConfig `json:"connectionMonitor"`
OrderManager OrderManager `json:"orderManager"`
DataHistoryManager DataHistoryManager `json:"dataHistoryManager"`
Expand Down Expand Up @@ -130,6 +137,25 @@ type CurrencyStateManager struct {
Delay time.Duration `json:"delay"`
}

// SyncManagerConfig stores the currency pair synchronization manager config
type SyncManagerConfig struct {
Enabled bool `json:"enabled"`
SynchronizeTicker bool `json:"synchronizeTicker"`
SynchronizeOrderbook bool `json:"synchronizeOrderbook"`
SynchronizeTrades bool `json:"synchronizeTrades"`
SynchronizeContinuously bool `json:"synchronizeContinuously"`
TimeoutREST time.Duration `json:"timeoutREST"`
TimeoutWebsocket time.Duration `json:"timeoutWebsocket"`
NumWorkers int `json:"numWorkers"`
FiatDisplayCurrency currency.Code `json:"fiatDisplayCurrency"`
PairFormatDisplay *currency.PairFormat `json:"pairFormatDisplay,omitempty"`
// log events
Verbose bool `json:"verbose"`
LogSyncUpdateEvents bool `json:"logSyncUpdateEvents"`
LogSwitchProtocolEvents bool `json:"logSwitchProtocolEvents"`
LogInitialSyncEvents bool `json:"logInitialSyncEvents"`
}

// ConnectionMonitorConfig defines the connection monitor variables to ensure
// that there is internet connectivity
type ConnectionMonitorConfig struct {
Expand Down
19 changes: 19 additions & 0 deletions config_example.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,25 @@
}
}
},
"syncManager": {
"enabled": true,
"synchronizeTicker": true,
"synchronizeOrderbook": true,
"synchronizeTrades": true,
"synchronizeContinuously": true,
"timeoutREST": 15000000000,
"timeoutWebsocket": 60000000000,
"numWorkers": 15,
"fiatDisplayCurrency": "USD",
"pairFormatDisplay": {
"uppercase": true,
"delimiter": "-"
},
"verbose": false,
"logSyncUpdateEvents": true,
"logSwitchProtocolEvents": true,
"logInitialSyncEvents": true
},
"connectionMonitor": {
"preferredDNSList": [
"8.8.8.8",
Expand Down
34 changes: 20 additions & 14 deletions engine/engine.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -490,22 +490,28 @@ func (bot *Engine) Start() error {
}
}

if bot.Settings.EnableExchangeSyncManager {
exchangeSyncCfg := &SyncManagerConfig{
SynchronizeTicker: bot.Settings.EnableTickerSyncing,
SynchronizeOrderbook: bot.Settings.EnableOrderbookSyncing,
SynchronizeTrades: bot.Settings.EnableTradeSyncing,
SynchronizeContinuously: bot.Settings.SyncContinuously,
TimeoutREST: bot.Settings.SyncTimeoutREST,
TimeoutWebsocket: bot.Settings.SyncTimeoutWebsocket,
NumWorkers: bot.Settings.SyncWorkersCount,
Verbose: bot.Settings.Verbose,
FiatDisplayCurrency: bot.Config.Currency.FiatDisplayCurrency,
PairFormatDisplay: bot.Config.Currency.CurrencyPairFormat,
if bot.Config.SyncManagerConfig.Enabled && bot.Settings.EnableExchangeSyncManager {
cfg := bot.Config.SyncManagerConfig
cfg.SynchronizeTicker = bot.Settings.EnableTickerSyncing && cfg.SynchronizeTicker
cfg.SynchronizeOrderbook = bot.Settings.EnableOrderbookSyncing && cfg.SynchronizeOrderbook
cfg.SynchronizeContinuously = bot.Settings.SyncContinuously && cfg.SynchronizeContinuously
cfg.SynchronizeTrades = bot.Settings.EnableTradeSyncing || cfg.SynchronizeTrades
cfg.Verbose = bot.Settings.Verbose || cfg.Verbose

if cfg.TimeoutREST != bot.Settings.SyncTimeoutREST &&
bot.Settings.SyncTimeoutREST != config.DefaultSyncerTimeoutREST {
cfg.TimeoutREST = bot.Settings.SyncTimeoutREST
}
if cfg.TimeoutWebsocket != bot.Settings.SyncTimeoutWebsocket &&
bot.Settings.SyncTimeoutWebsocket != config.DefaultSyncerTimeoutWebsocket {
cfg.TimeoutWebsocket = bot.Settings.SyncTimeoutWebsocket
}
if cfg.NumWorkers != bot.Settings.SyncWorkersCount &&
bot.Settings.SyncWorkersCount != config.DefaultSyncerWorkers {
cfg.NumWorkers = bot.Settings.SyncWorkersCount
}

if s, err := setupSyncManager(
exchangeSyncCfg,
&cfg,
bot.ExchangeManager,
&bot.Config.RemoteControl,
bot.Settings.EnableWebsocketRoutine,
Expand Down
30 changes: 19 additions & 11 deletions engine/helpers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,19 +185,27 @@ func (bot *Engine) SetSubsystem(subSystemName string, enable bool) error {
case SyncManagerName:
if enable {
if bot.currencyPairSyncer == nil {
exchangeSyncCfg := &SyncManagerConfig{
SynchronizeTicker: bot.Settings.EnableTickerSyncing,
SynchronizeOrderbook: bot.Settings.EnableOrderbookSyncing,
SynchronizeTrades: bot.Settings.EnableTradeSyncing,
SynchronizeContinuously: bot.Settings.SyncContinuously,
TimeoutREST: bot.Settings.SyncTimeoutREST,
TimeoutWebsocket: bot.Settings.SyncTimeoutWebsocket,
NumWorkers: bot.Settings.SyncWorkersCount,
FiatDisplayCurrency: bot.Config.Currency.FiatDisplayCurrency,
Verbose: bot.Settings.Verbose,
cfg := bot.Config.SyncManagerConfig
cfg.SynchronizeTicker = bot.Settings.EnableTickerSyncing && cfg.SynchronizeTicker
Copy link
Collaborator

Choose a reason for hiding this comment

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

When ticker sync is enabled in config this pattern used to override if set using flagSet.WithBool functionality in engine.

.\gocryptotrader -orderbooksync=true -tickersync=false

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Could you rephrase that please?

Copy link
Collaborator

Choose a reason for hiding this comment

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

When ticker sync is set to true in config, when I try to set it to false via command line it will not. I don't english well.

Copy link
Collaborator

Choose a reason for hiding this comment

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

For the inverse if I have both set to false in config and I run this command, it doesn't work:

 .\gocryptotrader --orderbooksync=true --tickersync=true

Copy link
Collaborator Author

@gloriousCode gloriousCode Aug 24, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Default app param values have now become false to address this. But then that will mean you can't disable it via command line.

I'm thinking to remove them based on what you're after

edit: still reworking

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Resolved, I forgot about the lovely existing helper functions

cfg.SynchronizeOrderbook = bot.Settings.EnableOrderbookSyncing && cfg.SynchronizeOrderbook
cfg.SynchronizeContinuously = bot.Settings.SyncContinuously && cfg.SynchronizeContinuously
cfg.SynchronizeTrades = bot.Settings.EnableTradeSyncing || cfg.SynchronizeTrades
cfg.Verbose = bot.Settings.Verbose || cfg.Verbose

if cfg.TimeoutREST != bot.Settings.SyncTimeoutREST &&
bot.Settings.SyncTimeoutREST != config.DefaultSyncerTimeoutREST {
cfg.TimeoutREST = bot.Settings.SyncTimeoutREST
}
if cfg.TimeoutWebsocket != bot.Settings.SyncTimeoutWebsocket &&
bot.Settings.SyncTimeoutWebsocket != config.DefaultSyncerTimeoutWebsocket {
cfg.TimeoutWebsocket = bot.Settings.SyncTimeoutWebsocket
}
if cfg.NumWorkers != bot.Settings.SyncWorkersCount &&
bot.Settings.SyncWorkersCount != config.DefaultSyncerWorkers {
cfg.NumWorkers = bot.Settings.SyncWorkersCount
}
bot.currencyPairSyncer, err = setupSyncManager(
exchangeSyncCfg,
&cfg,
bot.ExchangeManager,
&bot.Config.RemoteControl,
bot.Settings.EnableWebsocketRoutine)
Expand Down
Loading