*: remove tcp_port for tiflash >= 7.1.0 (#2220)

components/playground/instance/tiflash.go

@@ -138,7 +138,6 @@ func (inst *TiFlashInstance) Start(ctx context.Context, version utils.Version) e
 		fmt.Sprintf("--tmp_path=%s", filepath.Join(inst.Dir, "tmp")),
 		fmt.Sprintf("--path=%s", filepath.Join(inst.Dir, "data")),
 		fmt.Sprintf("--listen_host=%s", inst.Host),
-		fmt.Sprintf("--tcp_port=%d", inst.TCPPort),
 		fmt.Sprintf("--logger.log=%s", inst.LogFile()),
 		fmt.Sprintf("--logger.errorlog=%s", filepath.Join(inst.Dir, "tiflash_error.log")),
 		fmt.Sprintf("--status.metrics_port=%d", inst.StatusPort),

components/playground/instance/tiflash_pre7_config.go

@@ -34,10 +34,10 @@ const tiflashMarkCacheSizeOld = `mark_cache_size = 5368709120`
 const tiflashConfigOld = `
 default_profile = "default"
 display_name = "TiFlash"
-%[2]s
+http_port = %[2]d
 listen_host = "0.0.0.0"
-path = "%[5]s"
 tcp_port = %[3]d
+path = "%[5]s"
 tmp_path = "%[6]s"
 %[14]s
 %[13]s
@@ -109,11 +109,11 @@ func writeTiFlashConfigOld(w io.Writer, version utils.Version, tcpPort, httpPort
 	var conf string
 
 	if tidbver.TiFlashNotNeedSomeConfig(version.String()) {
-		conf = fmt.Sprintf(tiflashConfigOld, pdAddrs, fmt.Sprintf(`http_port = %d`, httpPort), tcpPort,
+		conf = fmt.Sprintf(tiflashConfigOld, pdAddrs, httpPort, tcpPort,
 			deployDir, dataDir, tmpDir, logDir, servicePort, metricsPort,
 			ip, strings.Join(tidbStatusAddrs, ","), clusterManagerPath, "", "")
 	} else {
-		conf = fmt.Sprintf(tiflashConfigOld, pdAddrs, fmt.Sprintf(`http_port = %d`, httpPort), tcpPort,
+		conf = fmt.Sprintf(tiflashConfigOld, pdAddrs, httpPort, tcpPort,
 			deployDir, dataDir, tmpDir, logDir, servicePort, metricsPort,
 			ip, strings.Join(tidbStatusAddrs, ","), clusterManagerPath, tiflashDaemonConfigOld, tiflashMarkCacheSizeOld)
 	}

embed/examples/cluster/minimal.yaml

@@ -182,6 +182,7 @@ tiflash_servers:
     # # SSH port of the server.
     # ssh_port: 22
     # # TiFlash TCP Service port.
+    # # Since 7.1.0, it is not actually listened, and only being used as part of the instance identity.
     # tcp_port: 9000
     # # TiFlash raft service and coprocessor service listening address.
     # flash_service_port: 3930

embed/examples/cluster/multi-dc.yaml

@@ -235,6 +235,7 @@ tiflash_servers:
     # # SSH port of the server.
     # ssh_port: 22
     # # TiFlash TCP Service port.
+    # # Since 7.1.0, it is not actually listened, and only being used as part of the instance identity.
     # tcp_port: 9000
     # # TiFlash raft service and coprocessor service listening address.
     # flash_service_port: 3930

embed/examples/cluster/topology.example.yaml

@@ -224,6 +224,7 @@ tiflash_servers:
     # # SSH port of the server.
     # ssh_port: 22
     # # TiFlash TCP Service port.
+    # # Since 7.1.0, it is not actually listened, and only being used as part of the instance identity.
     tcp_port: 9000
     # # TiFlash raft service and coprocessor service listening address.
     flash_service_port: 3930

pkg/cluster/ansible/service_test.go

@@ -23,7 +23,6 @@ default_profile = "default"
 display_name = "TiFlash"
 listen_host = "0.0.0.0"
 path = "/data1/test-cluster/leiysky-ansible-test-deploy/tiflash/data/db"
-tcp_port = 11315
 tmp_path = "/data1/test-cluster/leiysky-ansible-test-deploy/tiflash/data/db/tmp"
 
 [flash]

pkg/cluster/spec/tiflash.go

@@ -486,6 +486,11 @@ func (i *TiFlashInstance) initTiFlashConfig(ctx context.Context, clusterVersion
 			httpPort = fmt.Sprintf(`http_port: %d`, spec.HTTPPort)
 		}
 	}
+	tcpPort := "#"
+	// Config tcp_port is only required for TiFlash version < 7.1.0, and is recommended to not specify for TiFlash version >= 7.1.0.
+	if tidbver.TiFlashRequiresTCPPortConfig(clusterVersion) {
+		tcpPort = fmt.Sprintf(`tcp_port: %d`, spec.TCPPort)
+	}
 
 	// set TLS configs
 	spec.Config, err = i.setTLSConfig(ctx, enableTLS, spec.Config, paths)
@@ -511,7 +516,7 @@ server_configs:
     listen_host: "%[7]s"
     tmp_path: "%[11]s"
     %[1]s
-    tcp_port: %[3]d
+    %[3]s
     %[4]s
     flash.tidb_status_addr: "%[5]s"
     flash.service_addr: "%[6]s"
@@ -535,7 +540,7 @@ server_configs:
 `,
 		pathConfig,
 		paths.Log,
-		spec.TCPPort,
+		tcpPort,
 		httpPort,
 		strings.Join(tidbStatusAddrs, ","),
 		utils.JoinHostPort(spec.Host, spec.FlashServicePort),

pkg/tidbver/tidbver.go

@@ -66,6 +66,15 @@ func TiFlashNotNeedHTTPPortConfig(version string) bool {
 	return semver.Compare(version, "v7.1.0") >= 0 || strings.Contains(version, "nightly")
 }
 
+// TiFlashRequiresTCPPortConfig return if given version of TiFlash requires tcp_port config.
+// TiFlash 7.1.0 and later versions won't listen to tpc_port if the config is not given, which is recommended.
+// However this config is required for pre-7.1.0 versions because TiFlash will listen to it anyway,
+// and we must make sure the port is being configured as specified in the topology file,
+// otherwise multiple TiFlash instances will conflict.
+func TiFlashRequiresTCPPortConfig(version string) bool {
+	return semver.Compare(version, "v7.1.0") < 0
+}
+
 // TiFlashNotNeedSomeConfig return if given version of TiFlash do not need some config like runAsDaemon
 func TiFlashNotNeedSomeConfig(version string) bool {
 	// https://github.com/pingcap/tiup/pull/1673