YvCeung commented on code in PR #7952:
URL: https://github.com/apache/incubator-seata/pull/7952#discussion_r3009729102
##########
discovery/seata-discovery-raft/src/main/java/org/apache/seata/discovery/registry/raft/RaftRegistryServiceImpl.java:
##########
@@ -170,23 +203,20 @@ protected static void startQueryMetadata() {
new LinkedBlockingQueue<>(),
new NamedThreadFactory("refreshMetadata", 1,
true));
REFRESH_METADATA_EXECUTOR.execute(() -> {
- long metadataMaxAgeMs =
CONFIG.getLong(getMetadataMaxAgeMs(), 30000L);
+ long metadataMaxAgeMs =
CONFIG.getLong(getMetadataMaxAgeMs(), DEFAULT_METADATA_MAX_AGE_MS);
Review Comment:
`metadataMaxAgeMs` no longer seems to provide the same fallback freshness
guarantee after switching to the HTTP/2 watch path.
In the previous HTTP/1 implementation, the watch call was effectively
bounded by the long-poll timeout, so the background loop could return from
watch() roughly once every 30 seconds and re-check `metadataMaxAgeMs` at that
cadence.
With the new HTTP/2 path, the loop can block inside `watch.next()`, and the
current HTTP/2 read timeout is 300 seconds(`HTTP2_WATCH_READ_TIMEOUT_SECONDS`).
As a result, when there are no watch events, the fallback full refresh via
/metadata/v1/cluster may now be delayed for up to ~300 seconds.
So the concern here is not that HTTP/2 introduces blocking by itself, but
that it expands the maximum interval at which the fallback refresh logic can
run, from roughly 30 seconds to roughly 300 seconds. That looks like a
behavioral regression in metadata freshness semantics.
Would it make sense to preserve the original upper bound here? For example,
by ensuring the HTTP/2 watch loop wakes up at least once within
metadataMaxAgeMs, instead of depending on a much larger fixed stream read
timeout.
`metadataMaxAgeMs` 在切换到 HTTP/2 watch 路径后，似乎不再能提供与原来一致的兜底刷新语义。
在原来的 HTTP/1 实现中，watch 本质上受长轮询超时限制，客户端最多大约每 30 秒就会从 watch() 返回一次，因此后台循环至少能以约
30 秒的粒度重新检查一次 metadataMaxAgeMs。
但在新的 HTTP/2 路径中，线程会阻塞在 watch.next() 上，而当前 HTTP/2 的 read timeout 是 300
秒(`HTTP2_WATCH_READ_TIMEOUT_SECONDS`)。这意味着当没有 watch 事件时，基于 /metadata/v1/cluster
的兜底全量刷新可能最长延迟到约 300 秒后才会再次触发。
所以这里的核心问题不是 HTTP/2 会阻塞，而是它把兜底刷新逻辑的最大检查间隔，从原先大约 30 秒扩大到了现在的大约 300 秒。
是否可以考虑保留原有的时间上界？例如让 HTTP/2 watch 至少在 metadataMaxAgeMs 周期内被唤醒一次，而不是依赖一个更大的固定
stream read timeout。
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
