config: use fsnotify to do config file reload #191

xhebox · 2023-01-12T09:58:34Z

Signed-off-by: xhe xw897002528@gmail.com

What problem does this PR solve?

Issue Number: close #190

Problem Summary: The best online reload solution for tidb-operator. No more PR for tidb-operator after this adaption. It works forever.

What is changed: Mostly refactors in manager/config

cobra outputs to stderr, small fixes
remove a debugging fmt.Printf at logger in the previous PR
move TestBase/TestConcurrency to namespace_test.go
move e.get/set/list to namespace.go
tiproxyctl and HTTP api changed. config/set accepts toml returns json. config/get just returns toml.
config file is not read in cmd/tiproxy but responsible by manager/config
fix default values of workdir to $PWD/work
TestWatch is replaced by TestConfig

Check List

Tests

Unit test
Integration test
Manual test (add detailed scripts or steps below)

./bin/tiproxy --config conf/proxy.toml
edit it online
./bin/tiproxyctl config [get|set] online

No code

Notable changes

Has configuration change
Has HTTP API interfaces change (Don't forget to add the declarative for API)
Has tiproxyctl change
Other user behavior changes

Release note

Please refer to Release Notes Language Style Guide to write a quality release note.

None

Signed-off-by: xhe <xw897002528@gmail.com>

xhebox · 2023-01-12T09:59:00Z

cmd/tiproxy/main.go

+	rootCmd.SetOutput(os.Stdout)
+	rootCmd.SetErr(os.Stderr)


Cobra all output defaults to stderr.

xhebox · 2023-01-12T09:59:12Z

cmd/tiproxy/main.go

-	configFile := rootCmd.PersistentFlags().String("config", "", "proxy config file path")
-	logEncoder := rootCmd.PersistentFlags().String("log_encoder", "tidb", "log in format of tidb, console, or json")
-	logLevel := rootCmd.PersistentFlags().String("log_level", "", "log level")
-	_ = rootCmd.PersistentFlags().String("cluster_name", "tiproxy", "default cluster name, used to generate node name and differential clusters in dns discovery")


Remove useless cluster options.

lib/config/proxy.go

pkg/manager/config/config.go

xhebox · 2023-01-12T10:03:07Z

pkg/manager/config/manager_test.go

 	logger := logger.CreateLoggerForTest(t)

-	etcd_cfg := embed.NewConfig()


xhebox · 2023-01-12T10:03:25Z

pkg/manager/config/manager_test.go

 	})

 	t.Cleanup(cancel)

 	return cfgmgr, ctx
 }
-
-func TestBase(t *testing.T) {


Moved to namespace_test.go, no modification.

xhebox · 2023-01-12T10:03:55Z

pkg/manager/config/namespace.go

 )

+func (e *ConfigManager) get(ctx context.Context, ns, key string) (KVValue, error) {


Moved from manager.go

xhebox · 2023-01-12T10:04:31Z

pkg/proxy/proxy.go

+			select {
+			case <-ctx.Done():
+				return
+			case ach := <-cfgch:


listener.Accept() will block receiving changes from config channel, causing it blocking. Thus we do it in separate goroutines.

xhebox · 2023-01-12T10:08:26Z

pkg/server/server.go

-			return
-		}
+	// setup config manager
+	if err = srv.ConfigManager.Init(ctx, lg.Named("config"), sctx.ConfigFile, &sctx.Overlay); err != nil {


cfgmgr is inited much earlier to read the config file

pkg/manager/config/namespace_test.go

lib/config/proxy.go

pkg/manager/config/config.go

pkg/manager/config/config_test.go

Co-authored-by: djshow832 <zhangming@pingcap.com>

Signed-off-by: xhe <xw897002528@gmail.com>

pkg/manager/config/config.go

pkg/manager/config/manager.go

Signed-off-by: xhe <xw897002528@gmail.com>

config: use fsnotify to do config file reload

a4dac51

Signed-off-by: xhe <xw897002528@gmail.com>

xhebox force-pushed the dedicate_6 branch from aa70ca6 to a4dac51 Compare January 12, 2023 10:01

xhebox commented Jan 12, 2023

View reviewed changes

xhebox changed the title ~~[WIP] config: use fsnotify to do config file reload~~ config: use fsnotify to do config file reload Jan 12, 2023

xhebox requested a review from djshow832 January 12, 2023 10:10

djshow832 reviewed Jan 12, 2023

View reviewed changes

pkg/manager/config/namespace_test.go Outdated Show resolved Hide resolved

djshow832 reviewed Jan 12, 2023

View reviewed changes

xhebox mentioned this pull request Jan 12, 2023

api: add /api/debug/health #193

Merged

8 tasks

xhebox and others added 3 commits January 12, 2023 22:59

Update pkg/manager/config/namespace_test.go

e5208af

Co-authored-by: djshow832 <zhangming@pingcap.com>

*: re-watch dir to get rewatching working

1072617

Signed-off-by: xhe <xw897002528@gmail.com>

*: fix k8s

2b49eb9

Signed-off-by: xhe <xw897002528@gmail.com>

djshow832 reviewed Jan 13, 2023

View reviewed changes

pkg/manager/config/config.go Show resolved Hide resolved

pkg/manager/config/manager.go Outdated Show resolved Hide resolved

pkg/manager/config/manager.go Outdated Show resolved Hide resolved

pkg/manager/config/manager.go Outdated Show resolved Hide resolved

xhebox added 3 commits January 16, 2023 12:52

*: address comments

809179a

Signed-off-by: xhe <xw897002528@gmail.com>

Merge branch 'main' into dedicate_6

ec6cc26

*: fix tests

25bb0ff

Signed-off-by: xhe <xw897002528@gmail.com>

xhebox requested a review from djshow832 January 16, 2023 05:12

xhebox added 3 commits January 16, 2023 17:14

*: add comments

8922d4f

Signed-off-by: xhe <xw897002528@gmail.com>

*: clone config

8651639

Signed-off-by: xhe <xw897002528@gmail.com>

*: fix CI

5ee2dc8

Signed-off-by: xhe <xw897002528@gmail.com>

djshow832 approved these changes Jan 17, 2023

View reviewed changes

djshow832 merged commit e201bdf into pingcap:main Jan 17, 2023

xhebox mentioned this pull request Jan 17, 2023

manager: fix dynamic config update of tiproxy pingcap/tidb-operator#4825

Merged

10 tasks

xhebox deleted the dedicate_6 branch March 7, 2023 03:18

xhebox added a commit to xhebox/TiProxy that referenced this pull request Mar 7, 2023

config: use fsnotify to do config file reload (pingcap#191)

0673fe6

xhebox added a commit to xhebox/TiProxy that referenced this pull request Mar 13, 2023

config: use fsnotify to do config file reload (pingcap#191)

f4262b6

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

config: use fsnotify to do config file reload #191

config: use fsnotify to do config file reload #191

xhebox commented Jan 12, 2023 •

edited

Loading

xhebox Jan 12, 2023

xhebox Jan 12, 2023

xhebox Jan 12, 2023

xhebox Jan 12, 2023

xhebox Jan 12, 2023

xhebox Jan 12, 2023

xhebox Jan 12, 2023

		logger := logger.CreateLoggerForTest(t)

		etcd_cfg := embed.NewConfig()

		)

		func (e *ConfigManager) get(ctx context.Context, ns, key string) (KVValue, error) {

config: use fsnotify to do config file reload #191

config: use fsnotify to do config file reload #191

Conversation

xhebox commented Jan 12, 2023 • edited Loading

What problem does this PR solve?

Check List

Release note

xhebox Jan 12, 2023

Choose a reason for hiding this comment

xhebox Jan 12, 2023

Choose a reason for hiding this comment

xhebox Jan 12, 2023

Choose a reason for hiding this comment

xhebox Jan 12, 2023

Choose a reason for hiding this comment

xhebox Jan 12, 2023

Choose a reason for hiding this comment

xhebox Jan 12, 2023

Choose a reason for hiding this comment

xhebox Jan 12, 2023

Choose a reason for hiding this comment

xhebox commented Jan 12, 2023 •

edited

Loading