Repository: matrixorigin/matrixone
Pull request #7438 (2023-01-04 06:49:57)

all: rename S3 file service to Shared
## What type of PR is this?

- [ ] API-change
- [ ] BUG
- [x] Improvement
- [ ] Documentation
- [ ] Feature
- [ ] Test and CI
- [ ] Code Refactoring

## Which issue(s) this PR fixes:

issue #

## What this PR does / why we need it:
`S3` is a misleading file service name, since it may be backed by a local disk.
Rename it to `Shared`.
`S3` file services in old config files will be treated as `Shared`, so changing old config files is not required.


Patch:
diff --git a/cmd/mo-service/config.go b/cmd/mo-service/config.go
index cfc346bcb..8c329bcd8 100644
--- a/cmd/mo-service/config.go
+++ b/cmd/mo-service/config.go
@@ -154,6 +154,12 @@ func (c *Config) createFileService(defaultName string) (*fileservice.FileService
 	// create all services
 	services := make([]fileservice.FileService, 0, len(c.FileServices))
 	for _, config := range c.FileServices {
+
+		// for old config compatibility
+		if strings.EqualFold(config.Name, "s3") {
+			config.Name = defines.SharedFileServiceName
+		}
+
 		service, err := fileservice.NewFileService(config)
 		if err != nil {
 			return nil, err
@@ -182,8 +188,8 @@ func (c *Config) createFileService(defaultName string) (*fileservice.FileService
 		return nil, err
 	}
 
-	// ensure s3 exists
-	_, err = fileservice.Get[fileservice.FileService](fs, defines.S3FileServiceName)
+	// ensure shared exists
+	_, err = fileservice.Get[fileservice.FileService](fs, defines.SharedFileServiceName)
 	if err != nil {
 		return nil, err
 	}
diff --git a/etc/bootstrap-example2/dn-node-1.toml b/etc/bootstrap-example2/dn-node-1.toml
index 1d15172b4..1f38a606e 100644
--- a/etc/bootstrap-example2/dn-node-1.toml
+++ b/etc/bootstrap-example2/dn-node-1.toml
@@ -25,7 +25,7 @@ data-dir = "data-dir"
 
 [[fileservice]]
 # s3 fileservice instance, used to store data.
-name = "s3"
+name = "SHARED"
 # use disk as fileservice backend.
 backend = "DISK"
 # set the directory used by DISK backend. There must has a file named "thisisalocalfileservicedir"
diff --git a/etc/bootstrap-example2/dn-node-2.toml b/etc/bootstrap-example2/dn-node-2.toml
index c167f34cb..ff80a7e22 100644
--- a/etc/bootstrap-example2/dn-node-2.toml
+++ b/etc/bootstrap-example2/dn-node-2.toml
@@ -25,7 +25,7 @@ data-dir = "data-dir2"
 
 [[fileservice]]
 # s3 fileservice instance, used to store data.
-name = "s3"
+name = "SHARED"
 # use disk as fileservice backend.
 backend = "DISK"
 # set the directory used by DISK backend. There must has a file named "thisisalocalfileservicedir"
diff --git a/etc/cn-non-dist-memory.toml b/etc/cn-non-dist-memory.toml
index fb3c0a32c..581357f37 100644
--- a/etc/cn-non-dist-memory.toml
+++ b/etc/cn-non-dist-memory.toml
@@ -17,7 +17,7 @@ name = "LOCAL"
 backend = "MEM"
 
 [[fileservice]]
-name = "S3"
+name = "SHARED"
 backend = "MEM"
 
 [[fileservice]]
diff --git a/etc/cn-standalone-dist-tae.toml b/etc/cn-standalone-dist-tae.toml
index f2418d8c3..8bcc3e2df 100644
--- a/etc/cn-standalone-dist-tae.toml
+++ b/etc/cn-standalone-dist-tae.toml
@@ -44,7 +44,7 @@ name = "LOCAL"
 backend = "MEM"
 
 [[fileservice]]
-name = "S3"
+name = "SHARED"
 backend = "MEM"
 
 [[fileservice]]
diff --git a/etc/launch-tae-CN-tae-DN/cn.toml b/etc/launch-tae-CN-tae-DN/cn.toml
index 0f3b14a66..183f2573c 100644
--- a/etc/launch-tae-CN-tae-DN/cn.toml
+++ b/etc/launch-tae-CN-tae-DN/cn.toml
@@ -16,7 +16,7 @@ name = "LOCAL"
 backend = "DISK"
 
 [[fileservice]]
-name = "S3"
+name = "SHARED"
 backend = "DISK"
 data-dir = "mo-data/s3"
 
diff --git a/etc/launch-tae-CN-tae-DN/dn.toml b/etc/launch-tae-CN-tae-DN/dn.toml
index f5c7096b5..71fae35ad 100644
--- a/etc/launch-tae-CN-tae-DN/dn.toml
+++ b/etc/launch-tae-CN-tae-DN/dn.toml
@@ -16,7 +16,7 @@ name = "LOCAL"
 backend = "DISK"
 
 [[fileservice]]
-name = "S3"
+name = "SHARED"
 backend = "DISK"
 data-dir = "mo-data/s3"
 
diff --git a/etc/launch-tae-CN-tae-DN/log.toml b/etc/launch-tae-CN-tae-DN/log.toml
index 83b8a7b81..4a876db98 100644
--- a/etc/launch-tae-CN-tae-DN/log.toml
+++ b/etc/launch-tae-CN-tae-DN/log.toml
@@ -12,7 +12,7 @@ name = "LOCAL"
 backend = "DISK"
 
 [[fileservice]]
-name = "S3"
+name = "SHARED"
 backend = "DISK"
 data-dir = "mo-data/s3"
 
diff --git a/etc/launch-tae-compose/config/cn-0.toml b/etc/launch-tae-compose/config/cn-0.toml
index ee58b9581..db1820607 100644
--- a/etc/launch-tae-compose/config/cn-0.toml
+++ b/etc/launch-tae-compose/config/cn-0.toml
@@ -15,7 +15,7 @@ name = "LOCAL"
 
 [[fileservice]]
 backend = "MINIO"
-name = "S3"
+name = "SHARED"
 
 [fileservice.s3]
 bucket = "mo-test"
diff --git a/etc/launch-tae-compose/config/cn-1.toml b/etc/launch-tae-compose/config/cn-1.toml
index 96ef9db15..6b8fa1392 100644
--- a/etc/launch-tae-compose/config/cn-1.toml
+++ b/etc/launch-tae-compose/config/cn-1.toml
@@ -15,7 +15,7 @@ name = "LOCAL"
 
 [[fileservice]]
 backend = "MINIO"
-name = "S3"
+name = "SHARED"
 
 [fileservice.s3]
 bucket = "mo-test"
diff --git a/etc/launch-tae-compose/config/dn.toml b/etc/launch-tae-compose/config/dn.toml
index fc40a1a52..70f84f0f3 100644
--- a/etc/launch-tae-compose/config/dn.toml
+++ b/etc/launch-tae-compose/config/dn.toml
@@ -12,7 +12,7 @@ name = "LOCAL"
 
 [[fileservice]]
 backend = "MINIO"
-name = "S3"
+name = "SHARED"
 
 [fileservice.s3]
 bucket = "mo-test"
@@ -44,7 +44,7 @@ listen-address = "0.0.0.0:41010"
 
 [dn.Txn.Storage]
 backend = "TAE"
-fileservice = "S3"
+fileservice = "SHARED"
 log-backend = "logservice"
 
 [dn.Ckp]
diff --git a/etc/launch-tae-compose/config/log.toml b/etc/launch-tae-compose/config/log.toml
index 340715f29..38b97b1ee 100644
--- a/etc/launch-tae-compose/config/log.toml
+++ b/etc/launch-tae-compose/config/log.toml
@@ -13,7 +13,7 @@ name = "LOCAL"
 
 [[fileservice]]
 backend = "MINIO"
-name = "S3"
+name = "SHARED"
 
 [fileservice.s3]
 bucket = "mo-test"
diff --git a/etc/launch-tae-logservice/cn.toml b/etc/launch-tae-logservice/cn.toml
index 61d86ea61..bd54ae58d 100644
--- a/etc/launch-tae-logservice/cn.toml
+++ b/etc/launch-tae-logservice/cn.toml
@@ -18,7 +18,7 @@ name = "LOCAL"
 backend = "DISK"
 
 [[fileservice]]
-name = "S3"
+name = "SHARED"
 backend = "MEM"
 
 [[fileservice]]
diff --git a/etc/launch-tae-logservice/dn.toml b/etc/launch-tae-logservice/dn.toml
index 55a1b361f..c23208989 100644
--- a/etc/launch-tae-logservice/dn.toml
+++ b/etc/launch-tae-logservice/dn.toml
@@ -18,7 +18,7 @@ name = "LOCAL"
 backend = "DISK"
 
 [[fileservice]]
-name = "S3"
+name = "SHARED"
 backend = "MEM"
 
 [[fileservice]]
diff --git a/etc/launch-tae-logservice/log.toml b/etc/launch-tae-logservice/log.toml
index 82d936778..9dbfde9e9 100644
--- a/etc/launch-tae-logservice/log.toml
+++ b/etc/launch-tae-logservice/log.toml
@@ -12,7 +12,7 @@ name = "LOCAL"
 backend = "DISK"
 
 [[fileservice]]
-name = "S3"
+name = "SHARED"
 backend = "MEM"
 
 [[fileservice]]
diff --git a/etc/launch-tae-multi-CN-tae-DN/cn1.toml b/etc/launch-tae-multi-CN-tae-DN/cn1.toml
index fada7e8ca..04064f09a 100644
--- a/etc/launch-tae-multi-CN-tae-DN/cn1.toml
+++ b/etc/launch-tae-multi-CN-tae-DN/cn1.toml
@@ -16,7 +16,7 @@ name = "LOCAL"
 backend = "DISK"
 
 [[fileservice]]
-name = "S3"
+name = "SHARED"
 backend = "DISK"
 data-dir = "mo-data/s3"
 
diff --git a/etc/launch-tae-multi-CN-tae-DN/cn2.toml b/etc/launch-tae-multi-CN-tae-DN/cn2.toml
index 20c6b3e09..08bffc7ae 100644
--- a/etc/launch-tae-multi-CN-tae-DN/cn2.toml
+++ b/etc/launch-tae-multi-CN-tae-DN/cn2.toml
@@ -16,7 +16,7 @@ name = "LOCAL"
 backend = "DISK"
 
 [[fileservice]]
-name = "S3"
+name = "SHARED"
 backend = "DISK"
 data-dir = "mo-data/s3"
 
diff --git a/etc/launch-tae-multi-CN-tae-DN/dn.toml b/etc/launch-tae-multi-CN-tae-DN/dn.toml
index 7ed66e8ca..c7a7eb8b0 100644
--- a/etc/launch-tae-multi-CN-tae-DN/dn.toml
+++ b/etc/launch-tae-multi-CN-tae-DN/dn.toml
@@ -16,7 +16,7 @@ name = "LOCAL"
 backend = "DISK"
 
 [[fileservice]]
-name = "S3"
+name = "SHARED"
 backend = "DISK"
 data-dir = "mo-data/s3"
 
diff --git a/etc/launch-tae-multi-CN-tae-DN/log.toml b/etc/launch-tae-multi-CN-tae-DN/log.toml
index c61fad5e2..d73dea290 100644
--- a/etc/launch-tae-multi-CN-tae-DN/log.toml
+++ b/etc/launch-tae-multi-CN-tae-DN/log.toml
@@ -12,7 +12,7 @@ name = "LOCAL"
 backend = "DISK"
 
 [[fileservice]]
-name = "S3"
+name = "SHARED"
 backend = "DISK"
 data-dir = "mo-data/s3"
 
diff --git a/etc/launch/cn1.toml b/etc/launch/cn1.toml
index a5b57cd5a..2b9b3a2cc 100644
--- a/etc/launch/cn1.toml
+++ b/etc/launch/cn1.toml
@@ -19,7 +19,7 @@ name = "LOCAL"
 backend = "MEM"
 
 [[fileservice]]
-name = "S3"
+name = "SHARED"
 backend = "MEM"
 
 [[fileservice]]
diff --git a/etc/launch/dn1.toml b/etc/launch/dn1.toml
index fa3acac47..7f22a53eb 100644
--- a/etc/launch/dn1.toml
+++ b/etc/launch/dn1.toml
@@ -19,7 +19,7 @@ name = "LOCAL"
 backend = "MEM"
 
 [[fileservice]]
-name = "S3"
+name = "SHARED"
 backend = "MEM"
 
 [[fileservice]]
diff --git a/etc/launch/log1.toml b/etc/launch/log1.toml
index c6295f2fd..9a6438a6a 100644
--- a/etc/launch/log1.toml
+++ b/etc/launch/log1.toml
@@ -40,7 +40,7 @@ name = "LOCAL"
 backend = "MEM"
 
 [[fileservice]]
-name = "S3"
+name = "SHARED"
 backend = "MEM"
 
 [[fileservice]]
diff --git a/etc/launch/log2.toml b/etc/launch/log2.toml
index 7b23d4ec5..1e3731871 100644
--- a/etc/launch/log2.toml
+++ b/etc/launch/log2.toml
@@ -40,7 +40,7 @@ name = "LOCAL"
 backend = "MEM"
 
 [[fileservice]]
-name = "S3"
+name = "SHARED"
 backend = "MEM"
 
 [[fileservice]]
diff --git a/etc/launch/log3.toml b/etc/launch/log3.toml
index fbe6cda3d..410f718bb 100644
--- a/etc/launch/log3.toml
+++ b/etc/launch/log3.toml
@@ -40,7 +40,7 @@ name = "LOCAL"
 backend = "MEM"
 
 [[fileservice]]
-name = "S3"
+name = "SHARED"
 backend = "MEM"
 
 [[fileservice]]
diff --git a/pkg/cnservice/distributed_tae.go b/pkg/cnservice/distributed_tae.go
index e0c4f18a4..386009a83 100644
--- a/pkg/cnservice/distributed_tae.go
+++ b/pkg/cnservice/distributed_tae.go
@@ -49,7 +49,7 @@ func (s *service) initDistributedTAE(
 	}
 
 	// use s3 as main fs
-	fs, err := fileservice.Get[fileservice.FileService](s.fileService, defines.S3FileServiceName)
+	fs, err := fileservice.Get[fileservice.FileService](s.fileService, defines.SharedFileServiceName)
 	if err != nil {
 		return err
 	}
diff --git a/pkg/defines/const.go b/pkg/defines/const.go
index fed46dd0a..7b7f0a923 100644
--- a/pkg/defines/const.go
+++ b/pkg/defines/const.go
@@ -23,7 +23,7 @@ const (
 )
 
 const (
-	S3FileServiceName    = "S3"
-	LocalFileServiceName = "LOCAL"
-	ETLFileServiceName   = "ETL"
+	SharedFileServiceName = "SHARED"
+	LocalFileServiceName  = "LOCAL"
+	ETLFileServiceName    = "ETL"
 )
diff --git a/pkg/dnservice/factory.go b/pkg/dnservice/factory.go
index 1cc0a2052..53ce5aaad 100644
--- a/pkg/dnservice/factory.go
+++ b/pkg/dnservice/factory.go
@@ -136,7 +136,7 @@ func (s *store) newTAEStorage(shard metadata.DNShard, factory logservice.ClientF
 	}
 
 	// use s3 as main fs
-	fs, err := fileservice.Get[fileservice.FileService](s.fileService, defines.S3FileServiceName)
+	fs, err := fileservice.Get[fileservice.FileService](s.fileService, defines.SharedFileServiceName)
 	if err != nil {
 		return nil, err
 	}
diff --git a/pkg/logutil/adapt_s3.go b/pkg/logutil/adapt_s3.go
index 90874016d..334e1192b 100644
--- a/pkg/logutil/adapt_s3.go
+++ b/pkg/logutil/adapt_s3.go
@@ -23,7 +23,7 @@ import (
 )
 
 func GetS3Logger() logging.Logger {
-	logger := GetSkip1Logger().Named(defines.S3FileServiceName)
+	logger := GetSkip1Logger().Named(defines.SharedFileServiceName)
 	return &S3Logger{
 		Logger: logger,
 	}


Test patch:
diff --git a/cmd/mo-service/config_test.go b/cmd/mo-service/config_test.go
index 0e744f84d..ce8888c35 100644
--- a/cmd/mo-service/config_test.go
+++ b/cmd/mo-service/config_test.go
@@ -52,7 +52,7 @@ func TestParseDNConfig(t *testing.T) {
 	
 	[[fileservice]]
 	# s3 fileservice instance, used to store data.
-	name = "s3"
+	name = "SHARED"
 	# use disk as fileservice backend.
 	backend = "DISK"
 	# set the directory used by DISK backend. There must has a file named "thisisalocalfileservicedir"
@@ -69,7 +69,7 @@ func TestParseDNConfig(t *testing.T) {
 	assert.Equal(t, dnservice.StorageMEM, cfg.DN.Txn.Storage.Backend)
 	assert.Equal(t, 2, len(cfg.FileServices))
 	assert.Equal(t, "local", cfg.FileServices[0].Name)
-	assert.Equal(t, "s3", cfg.FileServices[1].Name)
+	assert.Equal(t, defines.SharedFileServiceName, cfg.FileServices[1].Name)
 	assert.Equal(t, 2, len(cfg.getDNServiceConfig().HAKeeper.ClientConfig.ServiceAddresses))
 }
 
@@ -84,7 +84,7 @@ func TestFileServiceFactory(t *testing.T) {
 		Backend: "MEM",
 	})
 	c.FileServices = append(c.FileServices, fileservice.Config{
-		Name:    defines.S3FileServiceName,
+		Name:    defines.SharedFileServiceName,
 		Backend: "MEM",
 	})
 	c.FileServices = append(c.FileServices, fileservice.Config{
diff --git a/etc/cn-standalone-dist-tae-test.toml b/etc/cn-standalone-dist-tae-test.toml
index daa3db885..bab2a5d0a 100644
--- a/etc/cn-standalone-dist-tae-test.toml
+++ b/etc/cn-standalone-dist-tae-test.toml
@@ -44,7 +44,7 @@ name = "LOCAL"
 backend = "MEM"
 
 [[fileservice]]
-name = "S3"
+name = "SHARED"
 backend = "MEM"
 
 [[fileservice]]
diff --git a/etc/cn-standalone-test.toml b/etc/cn-standalone-test.toml
index 9960b3042..44dbd2149 100644
--- a/etc/cn-standalone-test.toml
+++ b/etc/cn-standalone-test.toml
@@ -17,7 +17,7 @@ name = "LOCAL"
 backend = "MEM"
 
 [[fileservice]]
-name = "S3"
+name = "SHARED"
 backend = "MEM"
 
 [[fileservice]]
diff --git a/etc/dn-minimal-s3-test.toml b/etc/dn-minimal-s3-test.toml
index 930491b3d..2598f999a 100644
--- a/etc/dn-minimal-s3-test.toml
+++ b/etc/dn-minimal-s3-test.toml
@@ -23,7 +23,7 @@ data-dir = "data dir"
 
 [[fileservice]]
 # s3 fileservice instance, used to store data.
-name = "s3"
+name = "SHARED"
 # use s3 as fileservice backend.
 backend = "S3"
 [fileservice.s3]
diff --git a/etc/dn-minimal-test.toml b/etc/dn-minimal-test.toml
index 4fc6fb0c0..603d74ce2 100644
--- a/etc/dn-minimal-test.toml
+++ b/etc/dn-minimal-test.toml
@@ -23,7 +23,7 @@ data-dir = "data dir"
 
 [[fileservice]]
 # s3 fileservice instance, used to store data.
-name = "s3"
+name = "SHARED"
 # use disk as fileservice backend.
 backend = "DISK"
 # set the directory used by DISK backend. There must has a file named "thisisalocalfileservicedir"
diff --git a/etc/log-minimal-test.toml b/etc/log-minimal-test.toml
index bd4bd0530..14fca1fb3 100644
--- a/etc/log-minimal-test.toml
+++ b/etc/log-minimal-test.toml
@@ -33,7 +33,7 @@ backend = "DISK"
 data-dir = "store"
 
 [[fileservice]]
-name = "s3"
+name = "SHARED"
 backend = "DISK"
 data-dir = "store"
 
diff --git a/pkg/dnservice/store_test.go b/pkg/dnservice/store_test.go
index 54feacda2..9a34912b4 100644
--- a/pkg/dnservice/store_test.go
+++ b/pkg/dnservice/store_test.go
@@ -62,7 +62,7 @@ func TestStartWithReplicas(t *testing.T) {
 	assert.NoError(t, err)
 
 	factory := func(name string) (*fileservice.FileServices, error) {
-		s3fs, err := fileservice.NewMemoryFS(defines.S3FileServiceName)
+		s3fs, err := fileservice.NewMemoryFS(defines.SharedFileServiceName)
 		if err != nil {
 			return nil, err
 		}
@@ -149,7 +149,7 @@ func runDNStoreTest(
 			return nil, err
 		}
 		s3, err := fileservice.NewMemoryFS(
-			defines.S3FileServiceName,
+			defines.SharedFileServiceName,
 		)
 		if err != nil {
 			return nil, err
diff --git a/pkg/tests/service/fileservice.go b/pkg/tests/service/fileservice.go
index 2c5f4c2f4..ac4235981 100644
--- a/pkg/tests/service/fileservice.go
+++ b/pkg/tests/service/fileservice.go
@@ -73,7 +73,7 @@ func (c *testCluster) buildFileServices() *fileServices {
 		cnServiceNum: cnServiceNum,
 		dnLocalFSs:   dnLocals,
 		cnLocalFSs:   cnLocals,
-		s3FS:         factory(c.opt.rootDataDir, defines.S3FileServiceName),
+		s3FS:         factory(c.opt.rootDataDir, defines.SharedFileServiceName),
 	}
 }
 
diff --git a/pkg/testutil/util_new.go b/pkg/testutil/util_new.go
index 4aa9d5329..fe89520dc 100644
--- a/pkg/testutil/util_new.go
+++ b/pkg/testutil/util_new.go
@@ -27,6 +27,7 @@ import (
 	"github.com/matrixorigin/matrixone/pkg/container/nulls"
 	"github.com/matrixorigin/matrixone/pkg/container/types"
 	"github.com/matrixorigin/matrixone/pkg/container/vector"
+	"github.com/matrixorigin/matrixone/pkg/defines"
 	"github.com/matrixorigin/matrixone/pkg/fileservice"
 	"github.com/matrixorigin/matrixone/pkg/pb/timestamp"
 	"github.com/matrixorigin/matrixone/pkg/vm/process"
@@ -57,15 +58,15 @@ func NewProcessWithMPool(mp *mpool.MPool) *process.Process {
 var NewProc = NewProcess
 
 func NewFS() *fileservice.FileServices {
-	local, err := fileservice.NewMemoryFS("local")
+	local, err := fileservice.NewMemoryFS(defines.LocalFileServiceName)
 	if err != nil {
 		panic(err)
 	}
-	s3, err := fileservice.NewMemoryFS("s3")
+	s3, err := fileservice.NewMemoryFS(defines.SharedFileServiceName)
 	if err != nil {
 		panic(err)
 	}
-	etl, err := fileservice.NewMemoryFS("etl")
+	etl, err := fileservice.NewMemoryFS(defines.ETLFileServiceName)
 	if err != nil {
 		panic(err)
 	}