fix: Wrap init segcore tracing with golang timeout

See also #33483 Wrap `C.InitTrace` & `C.SetTrace` with timeout preventing otlp initializtion hangs forever when endpoint is not set correctly Signed-off-by: Congqi Xia <[email protected]>
milvus-io · May 30, 2024 · 726438e · 726438e
1 parent 8f46a20
commit 726438e
Show file tree

Hide file tree

Showing 3 changed files with 50 additions and 4 deletions.
diff --git a/internal/core/src/common/Tracer.cpp b/internal/core/src/common/Tracer.cpp
@@ -55,13 +55,13 @@ initTelemetry(const TraceConfig& cfg) {
         opts.transport_format = jaeger::TransportFormat::kThriftHttp;
         opts.endpoint = cfg.jaegerURL;
         exporter = jaeger::JaegerExporterFactory::Create(opts);
-        LOG_INFO("init jaeger exporter, endpoint:", opts.endpoint);
+        LOG_INFO("init jaeger exporter, endpoint: {}", opts.endpoint);
     } else if (cfg.exporter == "otlp") {
         auto opts = otlp::OtlpGrpcExporterOptions{};
         opts.endpoint = cfg.otlpEndpoint;
         opts.use_ssl_credentials = cfg.oltpSecure;
         exporter = otlp::OtlpGrpcExporterFactory::Create(opts);
-        LOG_INFO("init otlp exporter, endpoint:", opts.endpoint);
+        LOG_INFO("init otlp exporter, endpoint: {}", opts.endpoint);
     } else {
         LOG_INFO("Empty Trace");
         enable_trace = false;

diff --git a/internal/util/initcore/init_core.go b/internal/util/initcore/init_core.go
@@ -29,6 +29,7 @@ import "C"
 
 import (
 	"fmt"
+	"time"
 	"unsafe"
 
 	"github.com/cockroachdb/errors"
@@ -61,7 +62,13 @@ func InitTraceConfig(params *paramtable.ComponentParam) {
 		otlpEndpoint:   endpoint,
 		nodeID:         nodeID,
 	}
-	C.InitTrace(&config)
+	// oltp grpc may hangs forever, add timeout logic at go side
+	timeout := params.TraceCfg.InitTimeoutSeconds.GetAsDuration(time.Second)
+	callWithTimeout(func() {
+		C.InitTrace(&config)
+	}, func() {
+		panic("init segcore tracing timeout, See issue #33483")
+	}, timeout)
 }
 
 func ResetTraceConfig(params *paramtable.ComponentParam) {
@@ -81,7 +88,31 @@ func ResetTraceConfig(params *paramtable.ComponentParam) {
 		otlpEndpoint:   endpoint,
 		nodeID:         nodeID,
 	}
-	C.SetTrace(&config)
+
+	// oltp grpc may hangs forever, add timeout logic at go side
+	timeout := params.TraceCfg.InitTimeoutSeconds.GetAsDuration(time.Second)
+	callWithTimeout(func() {
+		C.SetTrace(&config)
+	}, func() {
+		panic("set segcore tracing timeout, See issue #33483")
+	}, timeout)
+}
+
+func callWithTimeout(fn func(), timeoutHandler func(), timeout time.Duration) {
+	if timeout > 0 {
+		ch := make(chan struct{})
+		go func() {
+			defer close(ch)
+			fn()
+		}()
+		select {
+		case <-ch:
+		case <-time.After(timeout):
+			timeoutHandler()
+		}
+	} else {
+		fn()
+	}
 }
 
 func InitRemoteChunkManager(params *paramtable.ComponentParam) error {

diff --git a/internal/util/initcore/init_core_test.go b/internal/util/initcore/init_core_test.go
@@ -20,6 +20,7 @@ import (
 	"testing"
 
 	"github.com/milvus-io/milvus/pkg/util/paramtable"
+	"github.com/stretchr/testify/assert"
 )
 
 func TestTracer(t *testing.T) {
@@ -29,3 +30,17 @@ func TestTracer(t *testing.T) {
 	paramtable.Get().Save(paramtable.Get().TraceCfg.Exporter.Key, "stdout")
 	ResetTraceConfig(paramtable.Get())
 }
+
+func TestOtlpHang(t *testing.T) {
+	paramtable.Init()
+	InitTraceConfig(paramtable.Get())
+
+	paramtable.Get().Save(paramtable.Get().TraceCfg.Exporter.Key, "otlp")
+	paramtable.Get().Save(paramtable.Get().TraceCfg.InitTimeoutSeconds.Key, "1")
+	defer paramtable.Get().Reset(paramtable.Get().TraceCfg.Exporter.Key)
+	defer paramtable.Get().Reset(paramtable.Get().TraceCfg.InitTimeoutSeconds.Key)
+
+	assert.Panics(t, func() {
+		ResetTraceConfig(paramtable.Get())
+	})
+}