You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@dubbo.apache.org by "zbchi (via GitHub)" <gi...@apache.org> on 2026/03/27 14:12:04 UTC

[PR] Feat/http3 negotiation fallback [dubbo-go]

zbchi opened a new pull request, #3281:
URL: https://github.com/apache/dubbo-go/pull/3281

   ### Description
   #### Before
   #3266 
   If HTTP/3 failed, the client could then fall back to HTTP/2 with the same `http.Request`.
   That behavior is risky for Triple, because request bodies come from `io.Pipe` or other non-replayable streams. 
   It also means a broken HTTP/3 path can keep being retried instead of letting the client stay on the healthy HTTP/2 path.
   
   #### After
   The client now keeps HTTP/2 as the stable path, learns about HTTP/3 from `Alt-Svc` on HTTP/2 responses, and validates the HTTP/3 path with an independent probe before switching later requests. Once HTTP/3 is known to be healthy, future requests may use it. If HTTP/3 later fails, the client does not replay the current request on HTTP/2; instead, it marks the HTTP/3 path unhealthy, enters cooldown, and keeps subsequent requests on HTTP/2 until it is time to probe again.
   
   This is also closer to the approach used in `dubbo`: treat HTTP/3 as an upgrade that must be validated out of band, rather than retrying the current business request across transports.
   
   in short, the behavior now is :
    1.  HTTP/2 remains the default and stable path.
    2. `Alt-Svc` on HTTP/2 responses makes the origin a candidate for HTTP/3.
    3. An independent HTTP/3 probe (`OPTIONS /`) is used to validate the path.
    4. Only future requests switch to HTTP/3 after the probe succeeds.
    5.  If an HTTP/3 business request fails, the current request is not replayed on HTTP/2.
    6. Failed HTTP/3 paths enter cooldown so the client does not keep retrying them immediately.
    7.  Caller-driven context cancellation/deadline errors do not mark the HTTP/3 path as unhealthy.
   
   
   ### Checklist
   - [x] I confirm the target branch is `develop`
   - [x] Code has passed local testing
   - [x] I have added tests that prove my fix is effective or that my feature works
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org


Re: [PR] feat: add stateful HTTP/3 negotiation with probe and cooldown [dubbo-go]

Posted by "AlexStocks (via GitHub)" <gi...@apache.org>.
AlexStocks commented on code in PR #3281:
URL: https://github.com/apache/dubbo-go/pull/3281#discussion_r3006220243


##########
protocol/triple/dual_transport.go:
##########
@@ -0,0 +1,353 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package triple
+
+import (
+	"context"
+	"crypto/tls"
+	"errors"
+	"io"
+	"net/http"
+	"net/url"
+	"sync"
+	"time"
+)
+
+import (
+	"github.com/dubbogo/gost/log/logger"
+
+	"github.com/quic-go/quic-go"
+	"github.com/quic-go/quic-go/http3"
+
+	"golang.org/x/net/http2"
+)
+
+import (
+	"dubbo.apache.org/dubbo-go/v3/common/constant"
+	tri "dubbo.apache.org/dubbo-go/v3/protocol/triple/triple_protocol"
+)
+
+type originMode int
+
+const (
+	// originUnknown means the origin has not advertised HTTP/3 yet.
+	originUnknown originMode = iota
+	// originCandidate means the origin advertised HTTP/3 and is waiting for validation.
+	originCandidate
+	// originProbing means an out-of-band HTTP/3 probe is in flight.
+	originProbing
+	// originH3Healthy means later requests may be sent over HTTP/3.
+	originH3Healthy
+	// originCooldown means HTTP/3 recently failed and should be avoided for a while.
+	originCooldown
+)
+
+const (
+	defaultH3ProbeTimeout = 3 * time.Second
+	defaultH3BaseCooldown = 4 * time.Second
+	defaultH3MaxCooldown  = 1 * time.Minute
+)
+
+// originState tracks whether the current upstream origin is ready for HTTP/3.
+type originState struct {
+	mode          originMode
+	failures      int
+	cooldownUntil time.Time
+}
+
+// dualTransport keeps HTTP/2 as the stable path and only uses HTTP/3 after discovery and probe.
+type dualTransport struct {
+	http2Transport http.RoundTripper
+	http3Transport http.RoundTripper
+	// Cache for alternative services to avoid repeated lookups
+	altSvcCache *tri.AltSvcCache
+
+	// state tracks HTTP/3 readiness for the bound upstream origin.
+	state originState
+
+	// mu protects state and serializes transitions between unknown/probing/healthy/cooldown.
+	mu sync.Mutex
+
+	probeTimeout time.Duration
+	baseCooldown time.Duration
+	maxCooldown  time.Duration
+}
+
+// newDualTransport creates a new dual transport that supports both HTTP/2 and HTTP/3
+func newDualTransport(tlsConfig *tls.Config, keepAliveInterval, keepAliveTimeout time.Duration) http.RoundTripper {
+	http2Transport := &http2.Transport{
+		TLSClientConfig: tlsConfig,
+		ReadIdleTimeout: keepAliveInterval,
+		PingTimeout:     keepAliveTimeout,
+	}
+
+	http3Transport := &http3.Transport{
+		TLSClientConfig: tlsConfig,
+		QUICConfig: &quic.Config{
+			KeepAlivePeriod: keepAliveInterval,
+			MaxIdleTimeout:  keepAliveTimeout,
+		},
+	}
+
+	return &dualTransport{
+		http2Transport: http2Transport,
+		http3Transport: http3Transport,
+		altSvcCache:    tri.NewAltSvcCache(),
+		probeTimeout:   defaultH3ProbeTimeout,
+		baseCooldown:   defaultH3BaseCooldown,
+		maxCooldown:    defaultH3MaxCooldown,
+	}
+}
+
+// RoundTrip implements http.RoundTripper interface with HTTP Alternative Services support
+func (dt *dualTransport) RoundTrip(req *http.Request) (*http.Response, error) {
+	host := req.URL.Host
+
+	if dt.shouldUseH3(host) {
+		// Only use HTTP/3 after a separate probe marks the origin healthy.
+		// If the HTTP/3 request fails, return the error directly instead of
+		// replaying the same request over HTTP/2 with a partially consumed body.
+		resp, err := dt.http3Transport.RoundTrip(req)
+		if err == nil {
+			dt.markH3Success(host)
+			return resp, nil
+		}
+		if dt.shouldMarkH3Failure(req, err) {
+			logger.Warnf("HTTP/3 request failed for %s: %v", req.URL.String(), err)
+			dt.markH3Failure(host)
+		}
+		return nil, err
+	}
+
+	resp, err := dt.http2Transport.RoundTrip(req)
+	if err != nil {
+		return nil, err
+	}
+
+	// Learn HTTP/3 availability from HTTP/2 responses and validate it with
+	// an independent probe before routing later requests over HTTP/3.
+	dt.observeH2Response(req.URL, resp.Header)
+	return resp, nil
+}
+
+// shouldUseH3 only decides whether the current request may use HTTP/3.
+func (dt *dualTransport) shouldUseH3(host string) bool {
+	if host == "" {
+		return false
+	}
+
+	altSvc := dt.altSvcCache.Get(host)
+	if altSvc == nil || altSvc.Protocol != constant.AltSvcProtocolH3 {
+		return false
+	}
+
+	now := time.Now()
+
+	dt.mu.Lock()
+	defer dt.mu.Unlock()
+
+	switch dt.state.mode {
+	case originH3Healthy:
+		return true
+
+	case originCooldown:
+		// Wait for the cooldown window to expire before probing HTTP/3 again.
+		if now.Before(dt.state.cooldownUntil) {
+			return false
+		}
+
+		logger.Debugf("HTTP/3 cooldown expired for %s", host)
+		dt.state.mode = originCandidate
+		dt.state.cooldownUntil = time.Time{}
+		return false
+
+	case originUnknown, originCandidate, originProbing:
+		return false
+	}
+	return false
+}
+
+func (dt *dualTransport) shouldMarkH3Failure(req *http.Request, err error) bool {
+	if err == nil {
+		return false
+	}
+	if req != nil && req.Context() != nil && req.Context().Err() != nil {
+		return false
+	}
+	if errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded) {
+		return false
+	}
+	return true
+}
+
+func (dt *dualTransport) markH3Success(host string) {
+	if host == "" {
+		return
+	}
+
+	dt.mu.Lock()
+	defer dt.mu.Unlock()
+
+	dt.state.mode = originH3Healthy
+	dt.state.failures = 0
+	dt.state.cooldownUntil = time.Time{}
+	logger.Debugf("HTTP/3 ready for %s", host)
+}
+
+func (dt *dualTransport) markH3Failure(host string) {
+	if host == "" {
+		return
+	}

Review Comment:
   [P0] RoundTrip 函数缺少对 req 参数的 nil 检查。
   
   直接访问 req.URL.Host,如果 req 或 req.URL 为 nil 会导致 panic。虽然 http.RoundTripper 接口规范要求 req 非 nil,但防御性编程应该添加检查。
   
   **改进建议:**
   ```go
   if req == nil || req.URL == nil {
       return nil, errors.New("invalid request")
   }
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org


Re: [PR] feat: add stateful HTTP/3 negotiation with probe and cooldown [dubbo-go]

Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #3281:
URL: https://github.com/apache/dubbo-go/pull/3281#issuecomment-4142930144

   ## [Codecov](https://app.codecov.io/gh/apache/dubbo-go/pull/3281?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
   :x: Patch coverage is `71.51515%` with `47 lines` in your changes missing coverage. Please review.
   :white_check_mark: Project coverage is 49.08%. Comparing base ([`fb8eff2`](https://app.codecov.io/gh/apache/dubbo-go/commit/fb8eff2f090851261e2d1acfc55414ede91ca9c0?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`80c6450`](https://app.codecov.io/gh/apache/dubbo-go/commit/80c64506395adffbeaddae3bee886755da2a7475?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
   
   | [Files with missing lines](https://app.codecov.io/gh/apache/dubbo-go/pull/3281?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
   |---|---|---|
   | [protocol/triple/dual\_transport.go](https://app.codecov.io/gh/apache/dubbo-go/pull/3281?src=pr&el=tree&filepath=protocol%2Ftriple%2Fdual_transport.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cHJvdG9jb2wvdHJpcGxlL2R1YWxfdHJhbnNwb3J0Lmdv) | 71.51% | [33 Missing and 14 partials :warning: ](https://app.codecov.io/gh/apache/dubbo-go/pull/3281?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   
   <details><summary>Additional details and impacted files</summary>
   
   
   
   ```diff
   @@             Coverage Diff             @@
   ##           develop    #3281      +/-   ##
   ===========================================
   + Coverage    48.96%   49.08%   +0.11%     
   ===========================================
     Files          467      468       +1     
     Lines        34006    34138     +132     
   ===========================================
   + Hits         16652    16755     +103     
   - Misses       16019    16034      +15     
   - Partials      1335     1349      +14     
   ```
   </details>
   
   [:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/dubbo-go/pull/3281?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).   
   :loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
   <details><summary> :rocket: New features to boost your workflow: </summary>
   
   - :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
   </details>


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org


Re: [PR] feat: add stateful HTTP/3 negotiation with probe and cooldown [dubbo-go]

Posted by "zbchi (via GitHub)" <gi...@apache.org>.
zbchi commented on code in PR #3281:
URL: https://github.com/apache/dubbo-go/pull/3281#discussion_r3007503508


##########
protocol/triple/dual_transport.go:
##########
@@ -0,0 +1,353 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package triple
+
+import (
+	"context"
+	"crypto/tls"
+	"errors"
+	"io"
+	"net/http"
+	"net/url"
+	"sync"
+	"time"
+)
+
+import (
+	"github.com/dubbogo/gost/log/logger"
+
+	"github.com/quic-go/quic-go"
+	"github.com/quic-go/quic-go/http3"
+
+	"golang.org/x/net/http2"
+)
+
+import (
+	"dubbo.apache.org/dubbo-go/v3/common/constant"
+	tri "dubbo.apache.org/dubbo-go/v3/protocol/triple/triple_protocol"
+)
+
+type originMode int
+
+const (
+	// originUnknown means the origin has not advertised HTTP/3 yet.
+	originUnknown originMode = iota
+	// originCandidate means the origin advertised HTTP/3 and is waiting for validation.
+	originCandidate
+	// originProbing means an out-of-band HTTP/3 probe is in flight.
+	originProbing
+	// originH3Healthy means later requests may be sent over HTTP/3.
+	originH3Healthy
+	// originCooldown means HTTP/3 recently failed and should be avoided for a while.
+	originCooldown
+)
+
+const (
+	defaultH3ProbeTimeout = 3 * time.Second
+	defaultH3BaseCooldown = 4 * time.Second
+	defaultH3MaxCooldown  = 1 * time.Minute
+)
+
+// originState tracks whether the current upstream origin is ready for HTTP/3.
+type originState struct {
+	mode          originMode
+	failures      int
+	cooldownUntil time.Time
+}
+
+// dualTransport keeps HTTP/2 as the stable path and only uses HTTP/3 after discovery and probe.
+type dualTransport struct {
+	http2Transport http.RoundTripper
+	http3Transport http.RoundTripper
+	// Cache for alternative services to avoid repeated lookups
+	altSvcCache *tri.AltSvcCache
+
+	// state tracks HTTP/3 readiness for the bound upstream origin.
+	state originState
+
+	// mu protects state and serializes transitions between unknown/probing/healthy/cooldown.
+	mu sync.Mutex
+
+	probeTimeout time.Duration
+	baseCooldown time.Duration
+	maxCooldown  time.Duration
+}
+
+// newDualTransport creates a new dual transport that supports both HTTP/2 and HTTP/3
+func newDualTransport(tlsConfig *tls.Config, keepAliveInterval, keepAliveTimeout time.Duration) http.RoundTripper {
+	http2Transport := &http2.Transport{
+		TLSClientConfig: tlsConfig,
+		ReadIdleTimeout: keepAliveInterval,
+		PingTimeout:     keepAliveTimeout,
+	}
+
+	http3Transport := &http3.Transport{
+		TLSClientConfig: tlsConfig,
+		QUICConfig: &quic.Config{
+			KeepAlivePeriod: keepAliveInterval,
+			MaxIdleTimeout:  keepAliveTimeout,
+		},
+	}
+
+	return &dualTransport{
+		http2Transport: http2Transport,
+		http3Transport: http3Transport,
+		altSvcCache:    tri.NewAltSvcCache(),
+		probeTimeout:   defaultH3ProbeTimeout,
+		baseCooldown:   defaultH3BaseCooldown,
+		maxCooldown:    defaultH3MaxCooldown,
+	}
+}
+
+// RoundTrip implements http.RoundTripper interface with HTTP Alternative Services support
+func (dt *dualTransport) RoundTrip(req *http.Request) (*http.Response, error) {
+	host := req.URL.Host
+
+	if dt.shouldUseH3(host) {
+		// Only use HTTP/3 after a separate probe marks the origin healthy.
+		// If the HTTP/3 request fails, return the error directly instead of
+		// replaying the same request over HTTP/2 with a partially consumed body.
+		resp, err := dt.http3Transport.RoundTrip(req)
+		if err == nil {
+			dt.markH3Success(host)
+			return resp, nil
+		}
+		if dt.shouldMarkH3Failure(req, err) {
+			logger.Warnf("HTTP/3 request failed for %s: %v", req.URL.String(), err)
+			dt.markH3Failure(host)
+		}
+		return nil, err
+	}
+
+	resp, err := dt.http2Transport.RoundTrip(req)
+	if err != nil {
+		return nil, err
+	}
+
+	// Learn HTTP/3 availability from HTTP/2 responses and validate it with
+	// an independent probe before routing later requests over HTTP/3.
+	dt.observeH2Response(req.URL, resp.Header)
+	return resp, nil
+}
+
+// shouldUseH3 only decides whether the current request may use HTTP/3.
+func (dt *dualTransport) shouldUseH3(host string) bool {
+	if host == "" {
+		return false
+	}
+
+	altSvc := dt.altSvcCache.Get(host)
+	if altSvc == nil || altSvc.Protocol != constant.AltSvcProtocolH3 {
+		return false
+	}
+
+	now := time.Now()
+
+	dt.mu.Lock()
+	defer dt.mu.Unlock()
+
+	switch dt.state.mode {
+	case originH3Healthy:
+		return true
+
+	case originCooldown:
+		// Wait for the cooldown window to expire before probing HTTP/3 again.
+		if now.Before(dt.state.cooldownUntil) {
+			return false
+		}
+
+		logger.Debugf("HTTP/3 cooldown expired for %s", host)
+		dt.state.mode = originCandidate
+		dt.state.cooldownUntil = time.Time{}
+		return false
+
+	case originUnknown, originCandidate, originProbing:
+		return false
+	}
+	return false
+}
+
+func (dt *dualTransport) shouldMarkH3Failure(req *http.Request, err error) bool {
+	if err == nil {
+		return false
+	}
+	if req != nil && req.Context() != nil && req.Context().Err() != nil {
+		return false
+	}
+	if errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded) {
+		return false
+	}
+	return true
+}
+
+func (dt *dualTransport) markH3Success(host string) {
+	if host == "" {
+		return
+	}
+
+	dt.mu.Lock()
+	defer dt.mu.Unlock()
+
+	dt.state.mode = originH3Healthy
+	dt.state.failures = 0
+	dt.state.cooldownUntil = time.Time{}
+	logger.Debugf("HTTP/3 ready for %s", host)
+}
+
+func (dt *dualTransport) markH3Failure(host string) {
+	if host == "" {
+		return
+	}
+
+	dt.mu.Lock()
+	defer dt.mu.Unlock()
+
+	dt.state.failures++
+	dt.state.mode = originCooldown
+	dt.state.cooldownUntil = time.Now().Add(dt.nextCooldown(dt.state.failures))
+	logger.Debugf(
+		"HTTP/3 cooldown for %s until %s after %d failure(s)",
+		host,
+		dt.state.cooldownUntil.Format(time.RFC3339),
+		dt.state.failures,
+	)
+}
+
+// observeH2Response learns Alt-Svc from the current HTTP/2 response for later requests.
+func (dt *dualTransport) observeH2Response(u *url.URL, headers http.Header) {
+	if u == nil || u.Host == "" {
+		return
+	}
+
+	dt.altSvcCache.UpdateFromHeaders(u.Host, headers)
+
+	altSvc := dt.altSvcCache.Get(u.Host)
+	if altSvc == nil || altSvc.Protocol != constant.AltSvcProtocolH3 {
+		return
+	}
+
+	now := time.Now()
+
+	dt.mu.Lock()
+	switch dt.state.mode {
+	case originUnknown:
+		dt.state.mode = originCandidate
+		logger.Debugf("HTTP/3 advertised by %s", u.Host)
+
+	case originCooldown:
+		if !now.Before(dt.state.cooldownUntil) {
+			dt.state.mode = originCandidate
+			dt.state.cooldownUntil = time.Time{}
+			logger.Debugf("HTTP/3 reprobe enabled for %s", u.Host)
+		}
+
+	case originCandidate, originProbing, originH3Healthy:
+		// Already aware of HTTP/3, nothing to do
+	}
+	dt.mu.Unlock()
+
+	dt.maybeStartProbe(u)
+}
+
+func (dt *dualTransport) maybeStartProbe(u *url.URL) {
+	if u == nil || u.Host == "" {
+		return
+	}
+	host := u.Host
+
+	altSvc := dt.altSvcCache.Get(host)
+	if altSvc == nil || altSvc.Protocol != constant.AltSvcProtocolH3 {
+		return
+	}
+
+	now := time.Now()
+
+	dt.mu.Lock()
+	switch dt.state.mode {
+	case originH3Healthy, originProbing:
+		dt.mu.Unlock()
+		return
+	case originCooldown:

Review Comment:
   go的 http.Request.Context() 返回值 always non-nil



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org


Re: [PR] feat: add stateful HTTP/3 negotiation with probe and cooldown [dubbo-go]

Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #3281:
URL: https://github.com/apache/dubbo-go/pull/3281#issuecomment-4152332719

   ## [![Quality Gate Passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-passed-20px.png 'Quality Gate Passed')](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3281) **Quality Gate passed**  
   Issues  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [10 New issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3281&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/accepted-16px.png '') [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3281&issueStatuses=ACCEPTED)
   
   Measures  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo-go&pullRequest=3281&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3281&metric=new_coverage&view=list)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [4.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3281&metric=new_duplicated_lines_density&view=list)  
     
   [See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3281)
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org


Re: [PR] feat: add stateful HTTP/3 negotiation with probe and cooldown [dubbo-go]

Posted by "zbchi (via GitHub)" <gi...@apache.org>.
zbchi commented on code in PR #3281:
URL: https://github.com/apache/dubbo-go/pull/3281#discussion_r3006113492


##########
protocol/triple/dual_transport.go:
##########
@@ -0,0 +1,350 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package triple
+
+import (
+	"context"
+	"crypto/tls"
+	"errors"
+	"io"
+	"net/http"
+	"net/url"
+	"sync"
+	"time"
+)
+
+import (
+	"github.com/dubbogo/gost/log/logger"
+
+	"github.com/quic-go/quic-go"
+	"github.com/quic-go/quic-go/http3"
+
+	"golang.org/x/net/http2"
+)
+
+import (
+	tri "dubbo.apache.org/dubbo-go/v3/protocol/triple/triple_protocol"
+)
+
+type originMode int
+
+const (
+	// originUnknown means the origin has not advertised HTTP/3 yet.
+	originUnknown originMode = iota
+	// originCandidate means the origin advertised HTTP/3 and is waiting for validation.
+	originCandidate
+	// originProbing means an out-of-band HTTP/3 probe is in flight.
+	originProbing
+	// originH3Healthy means later requests may be sent over HTTP/3.
+	originH3Healthy
+	// originCooldown means HTTP/3 recently failed and should be avoided for a while.
+	originCooldown
+)
+
+const (
+	defaultH3ProbeTimeout = 3 * time.Second
+	defaultH3BaseCooldown = 4 * time.Second
+	defaultH3MaxCooldown  = 1 * time.Minute
+)
+
+// originState tracks whether the current upstream origin is ready for HTTP/3.
+type originState struct {
+	mode          originMode
+	failures      int
+	cooldownUntil time.Time
+}
+
+// dualTransport keeps HTTP/2 as the stable path and only uses HTTP/3 after discovery and probe.
+type dualTransport struct {

Review Comment:
   in the current client wiring, one dualTransport is created for one clientManager / tri.Client and is effectively bound to one fixed upstream origin.   The Alt-Svc cache is host-keyed because Alt-Svc itself is host-scoped. So the cache being per-host is still correct even under the current single-origin transport usage.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org


Re: [PR] feat: add stateful HTTP/3 negotiation with probe and cooldown [dubbo-go]

Posted by "AlexStocks (via GitHub)" <gi...@apache.org>.
AlexStocks commented on code in PR #3281:
URL: https://github.com/apache/dubbo-go/pull/3281#discussion_r3006220926


##########
protocol/triple/dual_transport.go:
##########
@@ -0,0 +1,353 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package triple
+
+import (
+	"context"
+	"crypto/tls"
+	"errors"
+	"io"
+	"net/http"
+	"net/url"
+	"sync"
+	"time"
+)
+
+import (
+	"github.com/dubbogo/gost/log/logger"
+
+	"github.com/quic-go/quic-go"
+	"github.com/quic-go/quic-go/http3"
+
+	"golang.org/x/net/http2"
+)
+
+import (
+	"dubbo.apache.org/dubbo-go/v3/common/constant"
+	tri "dubbo.apache.org/dubbo-go/v3/protocol/triple/triple_protocol"
+)
+
+type originMode int
+
+const (
+	// originUnknown means the origin has not advertised HTTP/3 yet.
+	originUnknown originMode = iota
+	// originCandidate means the origin advertised HTTP/3 and is waiting for validation.
+	originCandidate
+	// originProbing means an out-of-band HTTP/3 probe is in flight.
+	originProbing
+	// originH3Healthy means later requests may be sent over HTTP/3.
+	originH3Healthy
+	// originCooldown means HTTP/3 recently failed and should be avoided for a while.
+	originCooldown
+)
+
+const (
+	defaultH3ProbeTimeout = 3 * time.Second
+	defaultH3BaseCooldown = 4 * time.Second
+	defaultH3MaxCooldown  = 1 * time.Minute
+)
+
+// originState tracks whether the current upstream origin is ready for HTTP/3.
+type originState struct {
+	mode          originMode
+	failures      int
+	cooldownUntil time.Time
+}
+
+// dualTransport keeps HTTP/2 as the stable path and only uses HTTP/3 after discovery and probe.
+type dualTransport struct {
+	http2Transport http.RoundTripper
+	http3Transport http.RoundTripper
+	// Cache for alternative services to avoid repeated lookups
+	altSvcCache *tri.AltSvcCache
+
+	// state tracks HTTP/3 readiness for the bound upstream origin.
+	state originState
+
+	// mu protects state and serializes transitions between unknown/probing/healthy/cooldown.
+	mu sync.Mutex
+
+	probeTimeout time.Duration
+	baseCooldown time.Duration
+	maxCooldown  time.Duration
+}
+
+// newDualTransport creates a new dual transport that supports both HTTP/2 and HTTP/3
+func newDualTransport(tlsConfig *tls.Config, keepAliveInterval, keepAliveTimeout time.Duration) http.RoundTripper {
+	http2Transport := &http2.Transport{
+		TLSClientConfig: tlsConfig,
+		ReadIdleTimeout: keepAliveInterval,
+		PingTimeout:     keepAliveTimeout,
+	}
+
+	http3Transport := &http3.Transport{
+		TLSClientConfig: tlsConfig,
+		QUICConfig: &quic.Config{
+			KeepAlivePeriod: keepAliveInterval,
+			MaxIdleTimeout:  keepAliveTimeout,
+		},
+	}
+
+	return &dualTransport{
+		http2Transport: http2Transport,
+		http3Transport: http3Transport,
+		altSvcCache:    tri.NewAltSvcCache(),
+		probeTimeout:   defaultH3ProbeTimeout,
+		baseCooldown:   defaultH3BaseCooldown,
+		maxCooldown:    defaultH3MaxCooldown,
+	}
+}
+
+// RoundTrip implements http.RoundTripper interface with HTTP Alternative Services support
+func (dt *dualTransport) RoundTrip(req *http.Request) (*http.Response, error) {
+	host := req.URL.Host
+
+	if dt.shouldUseH3(host) {
+		// Only use HTTP/3 after a separate probe marks the origin healthy.
+		// If the HTTP/3 request fails, return the error directly instead of
+		// replaying the same request over HTTP/2 with a partially consumed body.
+		resp, err := dt.http3Transport.RoundTrip(req)
+		if err == nil {
+			dt.markH3Success(host)
+			return resp, nil
+		}
+		if dt.shouldMarkH3Failure(req, err) {
+			logger.Warnf("HTTP/3 request failed for %s: %v", req.URL.String(), err)
+			dt.markH3Failure(host)
+		}
+		return nil, err
+	}
+
+	resp, err := dt.http2Transport.RoundTrip(req)
+	if err != nil {
+		return nil, err
+	}
+
+	// Learn HTTP/3 availability from HTTP/2 responses and validate it with
+	// an independent probe before routing later requests over HTTP/3.
+	dt.observeH2Response(req.URL, resp.Header)
+	return resp, nil
+}
+
+// shouldUseH3 only decides whether the current request may use HTTP/3.
+func (dt *dualTransport) shouldUseH3(host string) bool {
+	if host == "" {
+		return false
+	}
+
+	altSvc := dt.altSvcCache.Get(host)
+	if altSvc == nil || altSvc.Protocol != constant.AltSvcProtocolH3 {
+		return false
+	}
+
+	now := time.Now()
+
+	dt.mu.Lock()
+	defer dt.mu.Unlock()
+
+	switch dt.state.mode {
+	case originH3Healthy:
+		return true
+
+	case originCooldown:
+		// Wait for the cooldown window to expire before probing HTTP/3 again.
+		if now.Before(dt.state.cooldownUntil) {
+			return false
+		}
+
+		logger.Debugf("HTTP/3 cooldown expired for %s", host)
+		dt.state.mode = originCandidate
+		dt.state.cooldownUntil = time.Time{}
+		return false
+
+	case originUnknown, originCandidate, originProbing:
+		return false
+	}
+	return false
+}
+
+func (dt *dualTransport) shouldMarkH3Failure(req *http.Request, err error) bool {
+	if err == nil {
+		return false
+	}
+	if req != nil && req.Context() != nil && req.Context().Err() != nil {
+		return false
+	}
+	if errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded) {
+		return false
+	}
+	return true
+}
+
+func (dt *dualTransport) markH3Success(host string) {
+	if host == "" {
+		return
+	}
+
+	dt.mu.Lock()
+	defer dt.mu.Unlock()
+
+	dt.state.mode = originH3Healthy
+	dt.state.failures = 0
+	dt.state.cooldownUntil = time.Time{}
+	logger.Debugf("HTTP/3 ready for %s", host)
+}
+
+func (dt *dualTransport) markH3Failure(host string) {
+	if host == "" {
+		return
+	}
+
+	dt.mu.Lock()
+	defer dt.mu.Unlock()
+
+	dt.state.failures++
+	dt.state.mode = originCooldown
+	dt.state.cooldownUntil = time.Now().Add(dt.nextCooldown(dt.state.failures))
+	logger.Debugf(
+		"HTTP/3 cooldown for %s until %s after %d failure(s)",
+		host,
+		dt.state.cooldownUntil.Format(time.RFC3339),
+		dt.state.failures,
+	)
+}
+
+// observeH2Response learns Alt-Svc from the current HTTP/2 response for later requests.
+func (dt *dualTransport) observeH2Response(u *url.URL, headers http.Header) {
+	if u == nil || u.Host == "" {
+		return
+	}
+
+	dt.altSvcCache.UpdateFromHeaders(u.Host, headers)
+
+	altSvc := dt.altSvcCache.Get(u.Host)
+	if altSvc == nil || altSvc.Protocol != constant.AltSvcProtocolH3 {
+		return
+	}
+
+	now := time.Now()
+
+	dt.mu.Lock()
+	switch dt.state.mode {
+	case originUnknown:
+		dt.state.mode = originCandidate
+		logger.Debugf("HTTP/3 advertised by %s", u.Host)
+
+	case originCooldown:
+		if !now.Before(dt.state.cooldownUntil) {
+			dt.state.mode = originCandidate
+			dt.state.cooldownUntil = time.Time{}
+			logger.Debugf("HTTP/3 reprobe enabled for %s", u.Host)
+		}
+
+	case originCandidate, originProbing, originH3Healthy:
+		// Already aware of HTTP/3, nothing to do
+	}
+	dt.mu.Unlock()
+
+	dt.maybeStartProbe(u)
+}
+
+func (dt *dualTransport) maybeStartProbe(u *url.URL) {
+	if u == nil || u.Host == "" {
+		return
+	}
+	host := u.Host
+
+	altSvc := dt.altSvcCache.Get(host)
+	if altSvc == nil || altSvc.Protocol != constant.AltSvcProtocolH3 {
+		return
+	}
+
+	now := time.Now()
+
+	dt.mu.Lock()
+	switch dt.state.mode {
+	case originH3Healthy, originProbing:
+		dt.mu.Unlock()
+		return
+	case originCooldown:

Review Comment:
   [P1] Context 错误判断逻辑不完整。
   
   只检查了 req.Context().Err(),但没有检查 req.Context() 本身是否为 nil。如果 Context() 返回 nil,会导致 panic。
   
   **改进建议:**
   ```go
   if req != nil {
       if ctx := req.Context(); ctx != nil && ctx.Err() != nil {
           return false
       }
   }
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org


Re: [PR] feat: add stateful HTTP/3 negotiation with probe and cooldown [dubbo-go]

Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #3281:
URL: https://github.com/apache/dubbo-go/pull/3281#issuecomment-4142916591

   ## [![Quality Gate Passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-passed-20px.png 'Quality Gate Passed')](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3281) **Quality Gate passed**  
   Issues  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [9 New issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3281&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/accepted-16px.png '') [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3281&issueStatuses=ACCEPTED)
   
   Measures  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo-go&pullRequest=3281&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3281&metric=new_coverage&view=list)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3281&metric=new_duplicated_lines_density&view=list)  
     
   [See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3281)
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org


Re: [PR] feat: add stateful HTTP/3 negotiation with probe and cooldown [dubbo-go]

Posted by "Alanxtl (via GitHub)" <gi...@apache.org>.
Alanxtl merged PR #3281:
URL: https://github.com/apache/dubbo-go/pull/3281


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org


Re: [PR] feat: add stateful HTTP/3 negotiation with probe and cooldown [dubbo-go]

Posted by "Alanxtl (via GitHub)" <gi...@apache.org>.
Alanxtl commented on code in PR #3281:
URL: https://github.com/apache/dubbo-go/pull/3281#discussion_r3005714523


##########
protocol/triple/dual_transport.go:
##########
@@ -0,0 +1,350 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package triple
+
+import (
+	"context"
+	"crypto/tls"
+	"errors"
+	"io"
+	"net/http"
+	"net/url"
+	"sync"
+	"time"
+)
+
+import (
+	"github.com/dubbogo/gost/log/logger"
+
+	"github.com/quic-go/quic-go"
+	"github.com/quic-go/quic-go/http3"
+
+	"golang.org/x/net/http2"
+)
+
+import (
+	tri "dubbo.apache.org/dubbo-go/v3/protocol/triple/triple_protocol"
+)
+
+type originMode int
+
+const (
+	// originUnknown means the origin has not advertised HTTP/3 yet.
+	originUnknown originMode = iota
+	// originCandidate means the origin advertised HTTP/3 and is waiting for validation.
+	originCandidate
+	// originProbing means an out-of-band HTTP/3 probe is in flight.
+	originProbing
+	// originH3Healthy means later requests may be sent over HTTP/3.
+	originH3Healthy
+	// originCooldown means HTTP/3 recently failed and should be avoided for a while.
+	originCooldown
+)
+
+const (
+	defaultH3ProbeTimeout = 3 * time.Second
+	defaultH3BaseCooldown = 4 * time.Second
+	defaultH3MaxCooldown  = 1 * time.Minute
+)
+
+// originState tracks whether the current upstream origin is ready for HTTP/3.
+type originState struct {
+	mode          originMode
+	failures      int
+	cooldownUntil time.Time
+}
+
+// dualTransport keeps HTTP/2 as the stable path and only uses HTTP/3 after discovery and probe.
+type dualTransport struct {

Review Comment:
   `altSvcCache` is tracked per host, but `dualTransport` keeps only a single state originState for all requests. If this `RoundTripper` is ever reused across multiple origins, one host’s H3 success/cooldown can affect another host incorrectly. It seems the negotiation state should be keyed by host as well, or we should explicitly guarantee that one `dualTransport` instance is bound to exactly one origin.



##########
protocol/triple/dual_transport.go:
##########
@@ -0,0 +1,350 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package triple
+
+import (
+	"context"
+	"crypto/tls"
+	"errors"
+	"io"
+	"net/http"
+	"net/url"
+	"sync"
+	"time"
+)
+
+import (
+	"github.com/dubbogo/gost/log/logger"
+
+	"github.com/quic-go/quic-go"
+	"github.com/quic-go/quic-go/http3"
+
+	"golang.org/x/net/http2"
+)
+
+import (
+	tri "dubbo.apache.org/dubbo-go/v3/protocol/triple/triple_protocol"
+)
+
+type originMode int
+
+const (
+	// originUnknown means the origin has not advertised HTTP/3 yet.
+	originUnknown originMode = iota
+	// originCandidate means the origin advertised HTTP/3 and is waiting for validation.
+	originCandidate
+	// originProbing means an out-of-band HTTP/3 probe is in flight.
+	originProbing
+	// originH3Healthy means later requests may be sent over HTTP/3.
+	originH3Healthy
+	// originCooldown means HTTP/3 recently failed and should be avoided for a while.
+	originCooldown
+)
+
+const (
+	defaultH3ProbeTimeout = 3 * time.Second
+	defaultH3BaseCooldown = 4 * time.Second
+	defaultH3MaxCooldown  = 1 * time.Minute
+)
+
+// originState tracks whether the current upstream origin is ready for HTTP/3.
+type originState struct {
+	mode          originMode
+	failures      int
+	cooldownUntil time.Time
+}
+
+// dualTransport keeps HTTP/2 as the stable path and only uses HTTP/3 after discovery and probe.
+type dualTransport struct {
+	http2Transport http.RoundTripper
+	http3Transport http.RoundTripper
+	// Cache for alternative services to avoid repeated lookups
+	altSvcCache *tri.AltSvcCache
+
+	state originState
+
+	mu sync.Mutex
+
+	probeTimeout time.Duration
+	baseCooldown time.Duration
+	maxCooldown  time.Duration
+}
+
+// newDualTransport creates a new dual transport that supports both HTTP/2 and HTTP/3
+func newDualTransport(tlsConfig *tls.Config, keepAliveInterval, keepAliveTimeout time.Duration) http.RoundTripper {
+	http2Transport := &http2.Transport{
+		TLSClientConfig: tlsConfig,
+		ReadIdleTimeout: keepAliveInterval,
+		PingTimeout:     keepAliveTimeout,
+	}
+
+	http3Transport := &http3.Transport{
+		TLSClientConfig: tlsConfig,
+		QUICConfig: &quic.Config{
+			KeepAlivePeriod: keepAliveInterval,
+			MaxIdleTimeout:  keepAliveTimeout,
+		},
+	}
+
+	return &dualTransport{
+		http2Transport: http2Transport,
+		http3Transport: http3Transport,
+		altSvcCache:    tri.NewAltSvcCache(),
+		probeTimeout:   defaultH3ProbeTimeout,
+		baseCooldown:   defaultH3BaseCooldown,
+		maxCooldown:    defaultH3MaxCooldown,
+	}
+}
+
+// RoundTrip implements http.RoundTripper interface with HTTP Alternative Services support
+func (dt *dualTransport) RoundTrip(req *http.Request) (*http.Response, error) {
+	host := req.URL.Host
+
+	if dt.shouldUseH3(host) {
+		// Only use HTTP/3 after a separate probe marks the origin healthy.
+		// If the HTTP/3 request fails, return the error directly instead of
+		// replaying the same request over HTTP/2 with a partially consumed body.
+		resp, err := dt.http3Transport.RoundTrip(req)
+		if err == nil {
+			dt.markH3Success(host)
+			return resp, nil
+		}
+		if dt.shouldMarkH3Failure(req, err) {
+			logger.Warnf("HTTP/3 request failed for %s: %v", req.URL.String(), err)
+			dt.markH3Failure(host)
+		}
+		return nil, err
+	}
+
+	resp, err := dt.http2Transport.RoundTrip(req)
+	if err != nil {
+		return nil, err
+	}
+
+	// Learn HTTP/3 availability from HTTP/2 responses and validate it with
+	// an independent probe before routing later requests over HTTP/3.
+	dt.observeH2Response(req.URL, resp.Header)
+	return resp, nil
+}
+
+// shouldUseH3 only decides whether the current request may use HTTP/3.
+func (dt *dualTransport) shouldUseH3(host string) bool {
+	if host == "" {
+		return false
+	}
+
+	altSvc := dt.altSvcCache.Get(host)
+	if altSvc == nil || altSvc.Protocol != "h3" {

Review Comment:
   make "h3" into a const in common/consts



##########
protocol/triple/dual_transport.go:
##########
@@ -0,0 +1,350 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package triple
+
+import (
+	"context"
+	"crypto/tls"
+	"errors"
+	"io"
+	"net/http"
+	"net/url"
+	"sync"
+	"time"
+)
+
+import (
+	"github.com/dubbogo/gost/log/logger"
+
+	"github.com/quic-go/quic-go"
+	"github.com/quic-go/quic-go/http3"
+
+	"golang.org/x/net/http2"
+)
+
+import (
+	tri "dubbo.apache.org/dubbo-go/v3/protocol/triple/triple_protocol"
+)
+
+type originMode int
+
+const (
+	// originUnknown means the origin has not advertised HTTP/3 yet.
+	originUnknown originMode = iota
+	// originCandidate means the origin advertised HTTP/3 and is waiting for validation.
+	originCandidate
+	// originProbing means an out-of-band HTTP/3 probe is in flight.
+	originProbing
+	// originH3Healthy means later requests may be sent over HTTP/3.
+	originH3Healthy
+	// originCooldown means HTTP/3 recently failed and should be avoided for a while.
+	originCooldown
+)
+
+const (
+	defaultH3ProbeTimeout = 3 * time.Second
+	defaultH3BaseCooldown = 4 * time.Second
+	defaultH3MaxCooldown  = 1 * time.Minute
+)
+
+// originState tracks whether the current upstream origin is ready for HTTP/3.
+type originState struct {
+	mode          originMode
+	failures      int
+	cooldownUntil time.Time
+}
+
+// dualTransport keeps HTTP/2 as the stable path and only uses HTTP/3 after discovery and probe.
+type dualTransport struct {
+	http2Transport http.RoundTripper
+	http3Transport http.RoundTripper
+	// Cache for alternative services to avoid repeated lookups
+	altSvcCache *tri.AltSvcCache
+
+	state originState
+
+	mu sync.Mutex

Review Comment:
   it would also be good to add a test covering two different hosts sharing the same transport, to prove the negotiation/cooldown state is isolated correctly.



##########
protocol/triple/dual_transport.go:
##########
@@ -0,0 +1,350 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package triple
+
+import (
+	"context"
+	"crypto/tls"
+	"errors"
+	"io"
+	"net/http"
+	"net/url"
+	"sync"
+	"time"
+)
+
+import (
+	"github.com/dubbogo/gost/log/logger"
+
+	"github.com/quic-go/quic-go"
+	"github.com/quic-go/quic-go/http3"
+
+	"golang.org/x/net/http2"
+)
+
+import (
+	tri "dubbo.apache.org/dubbo-go/v3/protocol/triple/triple_protocol"
+)
+
+type originMode int
+
+const (
+	// originUnknown means the origin has not advertised HTTP/3 yet.
+	originUnknown originMode = iota
+	// originCandidate means the origin advertised HTTP/3 and is waiting for validation.
+	originCandidate
+	// originProbing means an out-of-band HTTP/3 probe is in flight.
+	originProbing
+	// originH3Healthy means later requests may be sent over HTTP/3.
+	originH3Healthy
+	// originCooldown means HTTP/3 recently failed and should be avoided for a while.
+	originCooldown
+)
+
+const (
+	defaultH3ProbeTimeout = 3 * time.Second
+	defaultH3BaseCooldown = 4 * time.Second
+	defaultH3MaxCooldown  = 1 * time.Minute
+)
+
+// originState tracks whether the current upstream origin is ready for HTTP/3.
+type originState struct {
+	mode          originMode
+	failures      int
+	cooldownUntil time.Time
+}
+
+// dualTransport keeps HTTP/2 as the stable path and only uses HTTP/3 after discovery and probe.
+type dualTransport struct {
+	http2Transport http.RoundTripper
+	http3Transport http.RoundTripper
+	// Cache for alternative services to avoid repeated lookups
+	altSvcCache *tri.AltSvcCache
+
+	state originState
+
+	mu sync.Mutex

Review Comment:
   pls write a comment to clearify what does this mu protect



##########
protocol/triple/dual_transport.go:
##########
@@ -0,0 +1,350 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package triple
+
+import (
+	"context"
+	"crypto/tls"
+	"errors"
+	"io"
+	"net/http"
+	"net/url"
+	"sync"
+	"time"
+)
+
+import (
+	"github.com/dubbogo/gost/log/logger"
+
+	"github.com/quic-go/quic-go"
+	"github.com/quic-go/quic-go/http3"
+
+	"golang.org/x/net/http2"
+)
+
+import (
+	tri "dubbo.apache.org/dubbo-go/v3/protocol/triple/triple_protocol"
+)
+
+type originMode int
+
+const (
+	// originUnknown means the origin has not advertised HTTP/3 yet.
+	originUnknown originMode = iota
+	// originCandidate means the origin advertised HTTP/3 and is waiting for validation.
+	originCandidate
+	// originProbing means an out-of-band HTTP/3 probe is in flight.
+	originProbing
+	// originH3Healthy means later requests may be sent over HTTP/3.
+	originH3Healthy
+	// originCooldown means HTTP/3 recently failed and should be avoided for a while.
+	originCooldown
+)
+
+const (
+	defaultH3ProbeTimeout = 3 * time.Second
+	defaultH3BaseCooldown = 4 * time.Second
+	defaultH3MaxCooldown  = 1 * time.Minute
+)
+
+// originState tracks whether the current upstream origin is ready for HTTP/3.
+type originState struct {
+	mode          originMode
+	failures      int
+	cooldownUntil time.Time
+}
+
+// dualTransport keeps HTTP/2 as the stable path and only uses HTTP/3 after discovery and probe.
+type dualTransport struct {
+	http2Transport http.RoundTripper
+	http3Transport http.RoundTripper
+	// Cache for alternative services to avoid repeated lookups
+	altSvcCache *tri.AltSvcCache
+
+	state originState
+
+	mu sync.Mutex

Review Comment:
   it would also be good to add a test covering two different hosts sharing the same transport, to prove the negotiation/cooldown state is isolated correctly.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org


Re: [PR] feat: add stateful HTTP/3 negotiation with probe and cooldown [dubbo-go]

Posted by "AlexStocks (via GitHub)" <gi...@apache.org>.
AlexStocks commented on code in PR #3281:
URL: https://github.com/apache/dubbo-go/pull/3281#discussion_r3006219806


##########
protocol/triple/dual_transport.go:
##########
@@ -0,0 +1,353 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package triple
+
+import (
+	"context"
+	"crypto/tls"
+	"errors"
+	"io"
+	"net/http"
+	"net/url"
+	"sync"
+	"time"
+)
+
+import (
+	"github.com/dubbogo/gost/log/logger"
+
+	"github.com/quic-go/quic-go"
+	"github.com/quic-go/quic-go/http3"
+
+	"golang.org/x/net/http2"
+)
+
+import (
+	"dubbo.apache.org/dubbo-go/v3/common/constant"
+	tri "dubbo.apache.org/dubbo-go/v3/protocol/triple/triple_protocol"
+)
+
+type originMode int
+
+const (
+	// originUnknown means the origin has not advertised HTTP/3 yet.
+	originUnknown originMode = iota
+	// originCandidate means the origin advertised HTTP/3 and is waiting for validation.
+	originCandidate
+	// originProbing means an out-of-band HTTP/3 probe is in flight.
+	originProbing
+	// originH3Healthy means later requests may be sent over HTTP/3.
+	originH3Healthy
+	// originCooldown means HTTP/3 recently failed and should be avoided for a while.
+	originCooldown
+)
+
+const (
+	defaultH3ProbeTimeout = 3 * time.Second
+	defaultH3BaseCooldown = 4 * time.Second
+	defaultH3MaxCooldown  = 1 * time.Minute
+)
+
+// originState tracks whether the current upstream origin is ready for HTTP/3.
+type originState struct {
+	mode          originMode
+	failures      int
+	cooldownUntil time.Time
+}
+
+// dualTransport keeps HTTP/2 as the stable path and only uses HTTP/3 after discovery and probe.
+type dualTransport struct {
+	http2Transport http.RoundTripper
+	http3Transport http.RoundTripper
+	// Cache for alternative services to avoid repeated lookups
+	altSvcCache *tri.AltSvcCache
+
+	// state tracks HTTP/3 readiness for the bound upstream origin.
+	state originState
+
+	// mu protects state and serializes transitions between unknown/probing/healthy/cooldown.
+	mu sync.Mutex
+
+	probeTimeout time.Duration
+	baseCooldown time.Duration
+	maxCooldown  time.Duration
+}
+
+// newDualTransport creates a new dual transport that supports both HTTP/2 and HTTP/3
+func newDualTransport(tlsConfig *tls.Config, keepAliveInterval, keepAliveTimeout time.Duration) http.RoundTripper {
+	http2Transport := &http2.Transport{
+		TLSClientConfig: tlsConfig,
+		ReadIdleTimeout: keepAliveInterval,
+		PingTimeout:     keepAliveTimeout,
+	}
+
+	http3Transport := &http3.Transport{
+		TLSClientConfig: tlsConfig,
+		QUICConfig: &quic.Config{
+			KeepAlivePeriod: keepAliveInterval,
+			MaxIdleTimeout:  keepAliveTimeout,
+		},
+	}
+
+	return &dualTransport{
+		http2Transport: http2Transport,
+		http3Transport: http3Transport,
+		altSvcCache:    tri.NewAltSvcCache(),
+		probeTimeout:   defaultH3ProbeTimeout,
+		baseCooldown:   defaultH3BaseCooldown,
+		maxCooldown:    defaultH3MaxCooldown,
+	}
+}
+
+// RoundTrip implements http.RoundTripper interface with HTTP Alternative Services support
+func (dt *dualTransport) RoundTrip(req *http.Request) (*http.Response, error) {
+	host := req.URL.Host
+
+	if dt.shouldUseH3(host) {
+		// Only use HTTP/3 after a separate probe marks the origin healthy.
+		// If the HTTP/3 request fails, return the error directly instead of
+		// replaying the same request over HTTP/2 with a partially consumed body.
+		resp, err := dt.http3Transport.RoundTrip(req)
+		if err == nil {
+			dt.markH3Success(host)
+			return resp, nil
+		}
+		if dt.shouldMarkH3Failure(req, err) {
+			logger.Warnf("HTTP/3 request failed for %s: %v", req.URL.String(), err)
+			dt.markH3Failure(host)
+		}
+		return nil, err
+	}
+
+	resp, err := dt.http2Transport.RoundTrip(req)
+	if err != nil {
+		return nil, err
+	}
+
+	// Learn HTTP/3 availability from HTTP/2 responses and validate it with
+	// an independent probe before routing later requests over HTTP/3.
+	dt.observeH2Response(req.URL, resp.Header)
+	return resp, nil
+}
+
+// shouldUseH3 only decides whether the current request may use HTTP/3.
+func (dt *dualTransport) shouldUseH3(host string) bool {
+	if host == "" {
+		return false
+	}
+
+	altSvc := dt.altSvcCache.Get(host)
+	if altSvc == nil || altSvc.Protocol != constant.AltSvcProtocolH3 {
+		return false
+	}
+
+	now := time.Now()
+
+	dt.mu.Lock()
+	defer dt.mu.Unlock()
+
+	switch dt.state.mode {
+	case originH3Healthy:
+		return true
+
+	case originCooldown:
+		// Wait for the cooldown window to expire before probing HTTP/3 again.
+		if now.Before(dt.state.cooldownUntil) {
+			return false
+		}
+
+		logger.Debugf("HTTP/3 cooldown expired for %s", host)
+		dt.state.mode = originCandidate
+		dt.state.cooldownUntil = time.Time{}
+		return false

Review Comment:
   [P0] dualTransport 结构体缺少对多 origin 场景的支持。
   
   当前 state 字段是单个 originState,这意味着整个 dualTransport 只能跟踪一个 origin 的状态。但实际使用中,一个 client 可能会请求多个不同的 host。
   
   **问题:**
   - 如果请求 hostA 和 hostB,它们会共享同一个 state
   - hostA 进入 cooldown 会影响 hostB 的 HTTP/3 使用
   
   **改进建议:**
   使用 map[string]*originState 为每个 origin 维护独立状态。



##########
protocol/triple/dual_transport.go:
##########
@@ -0,0 +1,353 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package triple
+
+import (
+	"context"
+	"crypto/tls"
+	"errors"
+	"io"
+	"net/http"
+	"net/url"
+	"sync"
+	"time"
+)
+
+import (
+	"github.com/dubbogo/gost/log/logger"
+
+	"github.com/quic-go/quic-go"
+	"github.com/quic-go/quic-go/http3"
+
+	"golang.org/x/net/http2"
+)
+
+import (
+	"dubbo.apache.org/dubbo-go/v3/common/constant"
+	tri "dubbo.apache.org/dubbo-go/v3/protocol/triple/triple_protocol"
+)
+
+type originMode int
+
+const (
+	// originUnknown means the origin has not advertised HTTP/3 yet.
+	originUnknown originMode = iota
+	// originCandidate means the origin advertised HTTP/3 and is waiting for validation.
+	originCandidate
+	// originProbing means an out-of-band HTTP/3 probe is in flight.
+	originProbing
+	// originH3Healthy means later requests may be sent over HTTP/3.
+	originH3Healthy
+	// originCooldown means HTTP/3 recently failed and should be avoided for a while.
+	originCooldown
+)
+
+const (
+	defaultH3ProbeTimeout = 3 * time.Second
+	defaultH3BaseCooldown = 4 * time.Second
+	defaultH3MaxCooldown  = 1 * time.Minute
+)
+
+// originState tracks whether the current upstream origin is ready for HTTP/3.
+type originState struct {
+	mode          originMode
+	failures      int
+	cooldownUntil time.Time
+}
+
+// dualTransport keeps HTTP/2 as the stable path and only uses HTTP/3 after discovery and probe.
+type dualTransport struct {
+	http2Transport http.RoundTripper
+	http3Transport http.RoundTripper
+	// Cache for alternative services to avoid repeated lookups
+	altSvcCache *tri.AltSvcCache
+
+	// state tracks HTTP/3 readiness for the bound upstream origin.
+	state originState
+
+	// mu protects state and serializes transitions between unknown/probing/healthy/cooldown.
+	mu sync.Mutex
+
+	probeTimeout time.Duration
+	baseCooldown time.Duration
+	maxCooldown  time.Duration
+}
+
+// newDualTransport creates a new dual transport that supports both HTTP/2 and HTTP/3
+func newDualTransport(tlsConfig *tls.Config, keepAliveInterval, keepAliveTimeout time.Duration) http.RoundTripper {
+	http2Transport := &http2.Transport{
+		TLSClientConfig: tlsConfig,
+		ReadIdleTimeout: keepAliveInterval,
+		PingTimeout:     keepAliveTimeout,
+	}
+
+	http3Transport := &http3.Transport{
+		TLSClientConfig: tlsConfig,
+		QUICConfig: &quic.Config{
+			KeepAlivePeriod: keepAliveInterval,
+			MaxIdleTimeout:  keepAliveTimeout,
+		},
+	}
+
+	return &dualTransport{
+		http2Transport: http2Transport,
+		http3Transport: http3Transport,
+		altSvcCache:    tri.NewAltSvcCache(),
+		probeTimeout:   defaultH3ProbeTimeout,
+		baseCooldown:   defaultH3BaseCooldown,
+		maxCooldown:    defaultH3MaxCooldown,
+	}
+}
+
+// RoundTrip implements http.RoundTripper interface with HTTP Alternative Services support
+func (dt *dualTransport) RoundTrip(req *http.Request) (*http.Response, error) {
+	host := req.URL.Host
+
+	if dt.shouldUseH3(host) {
+		// Only use HTTP/3 after a separate probe marks the origin healthy.
+		// If the HTTP/3 request fails, return the error directly instead of
+		// replaying the same request over HTTP/2 with a partially consumed body.
+		resp, err := dt.http3Transport.RoundTrip(req)
+		if err == nil {
+			dt.markH3Success(host)

Review Comment:
   [P1] Import 顺序违反 Go 代码规范。
   
   根据 Google Go Style Guide,import 应该按以下顺序分组:
   1. 标准库
   2. 第三方库  
   3. 项目内部库
   
   当前 line 127-134 的第三方库分成了多个 import 块,应该合并为一个。
   
   **改进建议:**
   将所有第三方库合并到一个 import 块中。



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org


Re: [PR] feat: add stateful HTTP/3 negotiation with probe and cooldown [dubbo-go]

Posted by "zbchi (via GitHub)" <gi...@apache.org>.
zbchi commented on code in PR #3281:
URL: https://github.com/apache/dubbo-go/pull/3281#discussion_r3007506967


##########
protocol/triple/dual_transport.go:
##########
@@ -0,0 +1,353 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package triple
+
+import (
+	"context"
+	"crypto/tls"
+	"errors"
+	"io"
+	"net/http"
+	"net/url"
+	"sync"
+	"time"
+)
+
+import (
+	"github.com/dubbogo/gost/log/logger"
+
+	"github.com/quic-go/quic-go"
+	"github.com/quic-go/quic-go/http3"
+
+	"golang.org/x/net/http2"
+)
+
+import (
+	"dubbo.apache.org/dubbo-go/v3/common/constant"
+	tri "dubbo.apache.org/dubbo-go/v3/protocol/triple/triple_protocol"
+)
+
+type originMode int
+
+const (
+	// originUnknown means the origin has not advertised HTTP/3 yet.
+	originUnknown originMode = iota
+	// originCandidate means the origin advertised HTTP/3 and is waiting for validation.
+	originCandidate
+	// originProbing means an out-of-band HTTP/3 probe is in flight.
+	originProbing
+	// originH3Healthy means later requests may be sent over HTTP/3.
+	originH3Healthy
+	// originCooldown means HTTP/3 recently failed and should be avoided for a while.
+	originCooldown
+)
+
+const (
+	defaultH3ProbeTimeout = 3 * time.Second
+	defaultH3BaseCooldown = 4 * time.Second
+	defaultH3MaxCooldown  = 1 * time.Minute
+)
+
+// originState tracks whether the current upstream origin is ready for HTTP/3.
+type originState struct {
+	mode          originMode
+	failures      int
+	cooldownUntil time.Time
+}
+
+// dualTransport keeps HTTP/2 as the stable path and only uses HTTP/3 after discovery and probe.
+type dualTransport struct {
+	http2Transport http.RoundTripper
+	http3Transport http.RoundTripper
+	// Cache for alternative services to avoid repeated lookups
+	altSvcCache *tri.AltSvcCache
+
+	// state tracks HTTP/3 readiness for the bound upstream origin.
+	state originState
+
+	// mu protects state and serializes transitions between unknown/probing/healthy/cooldown.
+	mu sync.Mutex
+
+	probeTimeout time.Duration
+	baseCooldown time.Duration
+	maxCooldown  time.Duration
+}
+
+// newDualTransport creates a new dual transport that supports both HTTP/2 and HTTP/3
+func newDualTransport(tlsConfig *tls.Config, keepAliveInterval, keepAliveTimeout time.Duration) http.RoundTripper {
+	http2Transport := &http2.Transport{
+		TLSClientConfig: tlsConfig,
+		ReadIdleTimeout: keepAliveInterval,
+		PingTimeout:     keepAliveTimeout,
+	}
+
+	http3Transport := &http3.Transport{
+		TLSClientConfig: tlsConfig,
+		QUICConfig: &quic.Config{
+			KeepAlivePeriod: keepAliveInterval,
+			MaxIdleTimeout:  keepAliveTimeout,
+		},
+	}
+
+	return &dualTransport{
+		http2Transport: http2Transport,
+		http3Transport: http3Transport,
+		altSvcCache:    tri.NewAltSvcCache(),
+		probeTimeout:   defaultH3ProbeTimeout,
+		baseCooldown:   defaultH3BaseCooldown,
+		maxCooldown:    defaultH3MaxCooldown,
+	}
+}
+
+// RoundTrip implements http.RoundTripper interface with HTTP Alternative Services support
+func (dt *dualTransport) RoundTrip(req *http.Request) (*http.Response, error) {
+	host := req.URL.Host
+
+	if dt.shouldUseH3(host) {
+		// Only use HTTP/3 after a separate probe marks the origin healthy.
+		// If the HTTP/3 request fails, return the error directly instead of
+		// replaying the same request over HTTP/2 with a partially consumed body.
+		resp, err := dt.http3Transport.RoundTrip(req)
+		if err == nil {
+			dt.markH3Success(host)
+			return resp, nil
+		}
+		if dt.shouldMarkH3Failure(req, err) {
+			logger.Warnf("HTTP/3 request failed for %s: %v", req.URL.String(), err)
+			dt.markH3Failure(host)
+		}
+		return nil, err
+	}
+
+	resp, err := dt.http2Transport.RoundTrip(req)
+	if err != nil {
+		return nil, err
+	}
+
+	// Learn HTTP/3 availability from HTTP/2 responses and validate it with
+	// an independent probe before routing later requests over HTTP/3.
+	dt.observeH2Response(req.URL, resp.Header)
+	return resp, nil
+}
+
+// shouldUseH3 only decides whether the current request may use HTTP/3.
+func (dt *dualTransport) shouldUseH3(host string) bool {
+	if host == "" {
+		return false
+	}
+
+	altSvc := dt.altSvcCache.Get(host)
+	if altSvc == nil || altSvc.Protocol != constant.AltSvcProtocolH3 {
+		return false
+	}
+
+	now := time.Now()
+
+	dt.mu.Lock()
+	defer dt.mu.Unlock()
+
+	switch dt.state.mode {
+	case originH3Healthy:
+		return true
+
+	case originCooldown:
+		// Wait for the cooldown window to expire before probing HTTP/3 again.
+		if now.Before(dt.state.cooldownUntil) {
+			return false
+		}
+
+		logger.Debugf("HTTP/3 cooldown expired for %s", host)
+		dt.state.mode = originCandidate
+		dt.state.cooldownUntil = time.Time{}
+		return false

Review Comment:
   每一个clientManager /tri 客户端实例都会创建一个独立的dualTransport,且该通道会固定绑定唯一的host



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org


Re: [PR] feat: add stateful HTTP/3 negotiation with probe and cooldown [dubbo-go]

Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #3281:
URL: https://github.com/apache/dubbo-go/pull/3281#issuecomment-4150057774

   ## [![Quality Gate Passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-passed-20px.png 'Quality Gate Passed')](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3281) **Quality Gate passed**  
   Issues  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [9 New issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3281&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/accepted-16px.png '') [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3281&issueStatuses=ACCEPTED)
   
   Measures  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo-go&pullRequest=3281&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3281&metric=new_coverage&view=list)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3281&metric=new_duplicated_lines_density&view=list)  
     
   [See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3281)
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org


Re: [PR] feat: add stateful HTTP/3 negotiation with probe and cooldown [dubbo-go]

Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #3281:
URL: https://github.com/apache/dubbo-go/pull/3281#issuecomment-4152692965

   ## [![Quality Gate Passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-passed-20px.png 'Quality Gate Passed')](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3281) **Quality Gate passed**  
   Issues  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [10 New issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3281&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/accepted-16px.png '') [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3281&issueStatuses=ACCEPTED)
   
   Measures  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo-go&pullRequest=3281&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3281&metric=new_coverage&view=list)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [4.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3281&metric=new_duplicated_lines_density&view=list)  
     
   [See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3281)
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org


Re: [PR] feat: add stateful HTTP/3 negotiation with probe and cooldown [dubbo-go]

Posted by "Alanxtl (via GitHub)" <gi...@apache.org>.
Alanxtl commented on code in PR #3281:
URL: https://github.com/apache/dubbo-go/pull/3281#discussion_r3005715572


##########
protocol/triple/dual_transport.go:
##########
@@ -0,0 +1,350 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package triple
+
+import (
+	"context"
+	"crypto/tls"
+	"errors"
+	"io"
+	"net/http"
+	"net/url"
+	"sync"
+	"time"
+)
+
+import (
+	"github.com/dubbogo/gost/log/logger"
+
+	"github.com/quic-go/quic-go"
+	"github.com/quic-go/quic-go/http3"
+
+	"golang.org/x/net/http2"
+)
+
+import (
+	tri "dubbo.apache.org/dubbo-go/v3/protocol/triple/triple_protocol"
+)
+
+type originMode int
+
+const (
+	// originUnknown means the origin has not advertised HTTP/3 yet.
+	originUnknown originMode = iota
+	// originCandidate means the origin advertised HTTP/3 and is waiting for validation.
+	originCandidate
+	// originProbing means an out-of-band HTTP/3 probe is in flight.
+	originProbing
+	// originH3Healthy means later requests may be sent over HTTP/3.
+	originH3Healthy
+	// originCooldown means HTTP/3 recently failed and should be avoided for a while.
+	originCooldown
+)
+
+const (
+	defaultH3ProbeTimeout = 3 * time.Second
+	defaultH3BaseCooldown = 4 * time.Second
+	defaultH3MaxCooldown  = 1 * time.Minute
+)
+
+// originState tracks whether the current upstream origin is ready for HTTP/3.
+type originState struct {
+	mode          originMode
+	failures      int
+	cooldownUntil time.Time
+}
+
+// dualTransport keeps HTTP/2 as the stable path and only uses HTTP/3 after discovery and probe.
+type dualTransport struct {

Review Comment:
   it would also be good to add a test covering two different hosts sharing the same transport, to prove the negotiation/cooldown state is isolated correctly.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org