Support deadline #1503
Support deadline #1503
Conversation
WalkthroughImplements RPC deadline propagation and enforcement across client, server, and context layers. Adds constants, context fields/methods, client-side deadline calculation/attachment, server-side deadline initialization, and integration tests across Bolt, HTTP/2, and Triple protocols. Minor Javadoc whitespace cleanup in ConsumerConfig. No public API signatures changed. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor App as Client App
participant AC as AbstractCluster (client)
participant Net as Network
participant Prov as ProviderInvoker (server)
participant Svc as Service Impl
App->>AC: invoke(request, timeout)
AC->>AC: read RpcInvokeContext.deadline
alt Upstream deadline exists
AC->>AC: remaining = deadline - now
alt remaining <= 0
AC-->>App: throw SofaTimeOutException (deadline exceeded)
else remaining > 0
AC->>AC: timeout = min(timeout, remaining)
AC->>Net: send(request + RPC_REQUEST_DEADLINE)
end
else No upstream deadline
opt CONFIG_KEY_DEADLINE_ENABLED == true
AC->>Net: send(request + RPC_REQUEST_DEADLINE=timeout)
end
opt deadline disabled
AC->>Net: send(request)
end
end
Net->>Prov: deliver(request)
Prov->>Prov: if enabled and deadline header present<br/>RpcInvokeContext.setDeadline(now + header)
Prov->>Svc: invoke business method
Svc-->>Prov: result
Prov-->>AC: response
AC-->>App: return result
sequenceDiagram
autonumber
actor App as Client App
participant AC as AbstractCluster
App->>AC: invoke(request)
AC->>AC: read RpcInvokeContext.deadline
AC->>AC: remaining = deadline - now
note over AC: remaining <= 0
AC-->>App: throw SofaTimeOutException (pre-send)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🔭 Outside diff range comments (1)
core/api/src/main/java/com/alipay/sofa/rpc/context/RpcInvokeContext.java (1)
118-146: Critical: Missing deadline field in clone method.The
clone()method copies thetimeoutfield (line 124) but fails to copy the newly addeddeadlinefield. This will cause deadline context to be lost during context cloning operations, potentially breaking deadline propagation in RPC call chains.Add the deadline field copying in the clone method:
//timeout child.setTimeout(parent.getTimeout()); +//deadline +child.setDeadline(parent.getDeadline()); //targetURL child.setTargetURL(parent.getTargetURL());
🧹 Nitpick comments (1)
test/test-integration/src/test/java/com/alipay/sofa/rpc/test/deadline/AbstractDeadlineChainTest.java (1)
245-245: Replace Thread.sleep with proper synchronizationUsing
Thread.sleep(1000)to wait for services to start is unreliable and could lead to race conditions.Consider using a more reliable synchronization mechanism like
CountDownLatchor polling with timeout:// Wait for services to be ready with timeout long timeout = System.currentTimeMillis() + 5000; while (System.currentTimeMillis() < timeout) { if (areServicesReady()) { break; } Thread.sleep(100); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
core-impl/client/src/main/java/com/alipay/sofa/rpc/client/AbstractCluster.java(4 hunks)core/api/src/main/java/com/alipay/sofa/rpc/client/ProviderInfoAttrs.java(1 hunks)core/api/src/main/java/com/alipay/sofa/rpc/common/RpcConstants.java(2 hunks)core/api/src/main/java/com/alipay/sofa/rpc/config/ConsumerConfig.java(4 hunks)core/api/src/main/java/com/alipay/sofa/rpc/config/MethodConfig.java(2 hunks)core/api/src/main/java/com/alipay/sofa/rpc/context/RpcInvokeContext.java(3 hunks)core/api/src/main/java/com/alipay/sofa/rpc/core/request/SofaRequest.java(2 hunks)core/api/src/main/java/com/alipay/sofa/rpc/filter/ProviderInvoker.java(1 hunks)core/api/src/test/java/com/alipay/sofa/rpc/config/ConsumerConfigTest.java(1 hunks)test/test-integration/src/test/java/com/alipay/sofa/rpc/test/deadline/AbstractDeadlineChainTest.java(1 hunks)test/test-integration/src/test/java/com/alipay/sofa/rpc/test/deadline/BoltDeadlineChainTest.java(1 hunks)test/test-integration/src/test/java/com/alipay/sofa/rpc/test/deadline/Http2DeadlineChainTest.java(1 hunks)test/test-integration/src/test/java/com/alipay/sofa/rpc/test/deadline/TripleDeadlineChainTest.java(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: in sofa rpc, tripletraceradapter.serverreceived() sets the invocation type on the sofarequest based ...
Learnt from: EvenLjj
PR: sofastack/sofa-rpc#1488
File: remoting/remoting-triple/src/main/java/com/alipay/sofa/rpc/interceptor/ServerReqHeaderInterceptor.java:105-120
Timestamp: 2025-05-08T06:59:23.963Z
Learning: In SOFA RPC, TripleTracerAdapter.serverReceived() sets the invocation type on the SofaRequest based on the gRPC method type (CLIENT_STREAMING, SERVER_STREAMING, BIDI_STREAMING), but the SofaRequest.isAsync() method does not directly recognize these streaming types as asynchronous. According to EvenLjj, some modification in TripleTracerAdapter.serverReceived makes sofaRequest.isAsync() return true for streaming requests.
Applied to files:
core-impl/client/src/main/java/com/alipay/sofa/rpc/client/AbstractCluster.java
🧬 Code Graph Analysis (3)
core/api/src/main/java/com/alipay/sofa/rpc/client/ProviderInfoAttrs.java (1)
core/api/src/main/java/com/alipay/sofa/rpc/common/RpcConstants.java (1)
RpcConstants(28-757)
core/api/src/main/java/com/alipay/sofa/rpc/config/ConsumerConfig.java (1)
core/api/src/main/java/com/alipay/sofa/rpc/common/RpcConstants.java (1)
RpcConstants(28-757)
test/test-integration/src/test/java/com/alipay/sofa/rpc/test/deadline/BoltDeadlineChainTest.java (1)
core/api/src/main/java/com/alipay/sofa/rpc/common/RpcConstants.java (1)
RpcConstants(28-757)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: sca
🔇 Additional comments (31)
core/api/src/main/java/com/alipay/sofa/rpc/client/ProviderInfoAttrs.java (2)
86-89: LGTM! Consistent constant addition.The new
ATTR_DEADLINEconstant follows the established patterns in the class and is properly placed in the dynamic configuration section with appropriate JavaDoc documentation.
86-89: LGTM! Clean and consistent constant addition.The new
ATTR_DEADLINEconstant follows the established pattern and is properly placed in the dynamic configuration section alongside similar attributes likeATTR_TIMEOUT.core/api/src/main/java/com/alipay/sofa/rpc/filter/ProviderInvoker.java (2)
96-101: LGTM! Proper deadline extraction and context setup.The deadline extraction logic correctly retrieves the deadline from request properties and converts it to an absolute timestamp for the invocation context. The null check ensures safe handling when no deadline is present.
97-101: LGTM! Correct deadline extraction and propagation.The implementation properly:
- Extracts the deadline from request properties using the correct constant
- Converts the relative deadline to an absolute timestamp by adding to current time
- Sets it in the invocation context for downstream deadline enforcement
The null check and placement before method invocation are appropriate.
core/api/src/main/java/com/alipay/sofa/rpc/common/RpcConstants.java (4)
410-413: LGTM! Consistent configuration constant addition.The new
CONFIG_KEY_DEADLINEconstant follows the established naming pattern and is properly placed with other configuration keys.
752-755: LGTM! Appropriate request header constant.The
RPC_REQUEST_DEADLINEconstant provides a well-named key for deadline propagation in requests. The "sofa_head_" prefix follows the framework's header naming convention.
410-413: LGTM! Well-defined configuration constant.The
CONFIG_KEY_DEADLINEconstant follows the established naming pattern and is appropriately placed alongside other configuration keys likeCONFIG_KEY_TIMEOUT.
752-755: LGTM! Appropriate request header constant.The
RPC_REQUEST_DEADLINEconstant uses a clear, prefixed name ("sofa_head_deadline") that avoids conflicts and clearly indicates its purpose for deadline propagation in RPC headers.core/api/src/main/java/com/alipay/sofa/rpc/config/MethodConfig.java (4)
50-53: LGTM! Consistent field definition.The
deadlinefield follows the same pattern as other configuration fields in the class with appropriate JavaDoc documentation.
158-175: LGTM! Proper getter/setter implementation.The getter and setter methods follow established patterns in the class. The getter returns -1 for null values, which is consistent with how other optional configurations are handled, and the setter maintains the builder pattern.
50-53: LGTM! Consistent field addition.The
deadlinefield is properly declared and documented, following the same pattern as other configuration fields liketimeout.
158-175: LGTM! Well-implemented accessor methods.The getter and setter methods follow established patterns:
- Getter returns -1 for null (appropriate for deadline semantics where -1 typically means "no deadline")
- Setter follows the builder pattern for method chaining
- Consistent with other similar methods in the class
core/api/src/main/java/com/alipay/sofa/rpc/core/request/SofaRequest.java (4)
167-170: LGTM! Consistent transient field definition.The
deadlinefield follows the same pattern as the existingtimeoutfield with appropriate transient modifier and JavaDoc documentation indicating its client-side usage.
288-306: LGTM! Standard getter/setter implementation.The getter and setter methods follow the established patterns in the class, with the setter maintaining the builder pattern by returning the SofaRequest instance for method chaining.
167-170: LGTM! Appropriately declared transient field.The
deadlinefield is properly declared as transient (correct for client-side invocation concerns) and well-documented. The placement after thetimeoutfield is logical.
288-306: LGTM! Clean accessor methods implementation.The getter and setter methods are correctly implemented:
- Getter returns the raw value (appropriate for request-level deadline vs configuration-level)
- Setter follows the builder pattern for method chaining
- Consistent with other similar methods in the class
test/test-integration/src/test/java/com/alipay/sofa/rpc/test/deadline/BoltDeadlineChainTest.java (2)
27-37: LGTM - Clean protocol-specific implementation.The class properly extends the abstract test base and correctly overrides the required methods with appropriate Bolt-specific values.
39-42: LGTM - Standard test delegation pattern.The test method correctly delegates to the parent class's shared test logic, following the expected pattern for protocol-specific test implementations.
core/api/src/test/java/com/alipay/sofa/rpc/config/ConsumerConfigTest.java (1)
57-80: LGTM - Comprehensive deadline timeout testing.The test method thoroughly validates the deadline timeout resolution logic:
- Global deadline fallback behavior
- Method-level configuration inheritance
- Method-specific deadline override
- Zero deadline value handling
The test structure mirrors the existing
testMethodTimeoutpattern, ensuring consistency in the test suite.core/api/src/main/java/com/alipay/sofa/rpc/config/ConsumerConfig.java (4)
227-230: LGTM - Consistent field declaration.The deadline field follows established patterns in the class with appropriate default value (-1) and positioning near related timeout configuration.
806-824: LGTM - Standard getter/setter implementation.The deadline accessor methods follow the established patterns in the class with proper fluent interface design and consistent documentation.
968-980: LGTM - Correct method-level deadline resolution.The method properly implements deadline timeout resolution with appropriate fallback logic from method-specific to global configuration. The implementation follows the established pattern of similar methods in the class.
Note: The fallback condition uses -1 (vs timeout's use of 0), which appears intentional given different semantics for deadline vs timeout configuration.
666-666: Minor documentation formatting update.The JavaDoc parameter documentation has been reformatted but maintains the same content.
test/test-integration/src/test/java/com/alipay/sofa/rpc/test/deadline/Http2DeadlineChainTest.java (3)
34-42: LGTM - Appropriate HTTP/2 protocol configuration.The protocol type and base port overrides are correctly implemented for HTTP/2 (h2c) testing with appropriate port range separation from other protocols.
44-53: LGTM - Proper HTTP/2 protocol-specific configuration.The consumer configuration method correctly handles HTTP/2-specific requirements by converting the URL scheme to
h2c://and setting the appropriate protocol. The documentation clearly explains the special handling needed for HTTP/2.
55-58: LGTM - Standard test delegation pattern.The test method follows the established pattern for protocol-specific deadline chain testing.
core/api/src/main/java/com/alipay/sofa/rpc/context/RpcInvokeContext.java (4)
52-55: LGTM - Appropriate deadline field declaration.The Long type is suitable for timestamp values, and null default provides clear "not set" semantics consistent with the context's design patterns.
193-211: LGTM - Standard accessor implementation.The getter and setter methods follow the established patterns in the class with appropriate return types and documentation.
213-218: LGTM - Correct deadline timeout logic.The method properly checks for deadline expiration with appropriate null safety and correct time comparison logic.
495-495: LGTM - Consistent toString formatting.The deadline field is properly included in the string representation with consistent formatting.
test/test-integration/src/test/java/com/alipay/sofa/rpc/test/deadline/TripleDeadlineChainTest.java (1)
26-62: LGTM!The Triple protocol-specific test implementation is clean and follows the expected pattern. The protocol configuration and URL transformation are handled correctly.
core-impl/client/src/main/java/com/alipay/sofa/rpc/client/AbstractCluster.java
Show resolved
Hide resolved
core-impl/client/src/main/java/com/alipay/sofa/rpc/client/AbstractCluster.java
Show resolved
Hide resolved
core-impl/client/src/main/java/com/alipay/sofa/rpc/client/AbstractCluster.java
Outdated
Show resolved
Hide resolved
...t-integration/src/test/java/com/alipay/sofa/rpc/test/deadline/AbstractDeadlineChainTest.java
Outdated
Show resolved
Hide resolved
...t-integration/src/test/java/com/alipay/sofa/rpc/test/deadline/AbstractDeadlineChainTest.java
Outdated
Show resolved
Hide resolved
...t-integration/src/test/java/com/alipay/sofa/rpc/test/deadline/AbstractDeadlineChainTest.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (4)
test/test-integration/src/test/java/com/alipay/sofa/rpc/test/deadline/AbstractDeadlineChainTest.java (4)
331-341: Make timing assertions stable: use nanoTime and tolerance; unrefer consumer after testUse System.nanoTime for elapsed time, assert within a tolerance range, and unRefer the per-test ConsumerConfig to avoid leaking client resources.
- ServiceA serviceA = consumerConfig.refer(); - - long startTime = System.currentTimeMillis(); - String result = serviceA.processA("test-message"); - long endTime = System.currentTimeMillis(); - - Assert.assertNotNull(result); - Assert.assertTrue(result.contains("ServiceA-ServiceB-ServiceC-test-message")); - Assert.assertTrue(endTime - startTime >= 13000); // 至少13秒 - Assert.assertTrue(endTime - startTime < 20000); // 小于deadline + ServiceA serviceA = consumerConfig.refer(); + long start = System.nanoTime(); + try { + String result = serviceA.processA("test-message"); + long elapsedMs = (System.nanoTime() - start) / 1_000_000L; + Assert.assertNotNull(result); + Assert.assertTrue(result.contains("ServiceA-ServiceB-ServiceC-test-message")); + // 13s chain; allow ±15% tolerance and ensure below deadline buffer + Assert.assertTrue("elapsed: " + elapsedMs, elapsedMs >= 11_000); + Assert.assertTrue("elapsed: " + elapsedMs, elapsedMs < 22_000); + } finally { + consumerConfig.unRefer(); + }Additionally, consider asserting exact expected string equality if formatting is stable.
346-377: Deadline-timeout test: stabilize timing and verify ServiceC non-start after failure; unrefer consumerRefine timing bounds with tolerance, assert after invocation that C did not start (with the AtomicBoolean change), and unrefer the consumer.
- ServiceA serviceA = consumerConfig.refer(); - Assert.assertFalse(isServiceCStarted.get()); - - long startTime = System.currentTimeMillis(); + ServiceA serviceA = consumerConfig.refer(); + long start = System.nanoTime(); boolean error = false; try { serviceA.processA("test-message"); Assert.fail("Should throw timeout exception"); } catch (Exception e) { - long endTime = System.currentTimeMillis(); + long elapsedMs = (System.nanoTime() - start) / 1_000_000L; Assert.assertTrue(e instanceof SofaTimeOutException); - Assert.assertTrue(endTime - startTime <= 9000); + // 4s deadline; allow tolerance and RPC overhead + Assert.assertTrue("elapsed: " + elapsedMs, elapsedMs >= 3_000); + Assert.assertTrue("elapsed: " + elapsedMs, elapsedMs <= 9_000); + // ensure C didn't start + Assert.assertFalse("ServiceC should not start when deadline expires before C", serviceCStarted.get()); error = true; - } + } finally { + consumerConfig.unRefer(); + } Assert.assertTrue(error);
398-415: Upstream deadline propagation: stabilize timing; clear context done; unrefer consumerUse nanoTime and tolerant bounds; also unrefer the consumer created in this method.
- ServiceA serviceA = consumerConfig.refer(); + ServiceA serviceA = consumerConfig.refer(); @@ - long startTime = System.currentTimeMillis(); + long start = System.nanoTime(); boolean error = false; try { serviceA.processA("test-message"); Assert.fail("Should throw timeout exception due to upstream deadline"); } catch (Exception e) { - long endTime = System.currentTimeMillis(); + long elapsedMs = (System.nanoTime() - start) / 1_000_000L; Assert.assertTrue(e instanceof SofaTimeOutException); - Assert.assertTrue(endTime - startTime <= 10000); // 应该在8秒附近超时 + // Upstream deadline ~8s; allow tolerance and RPC overhead + Assert.assertTrue("elapsed: " + elapsedMs, elapsedMs >= 6_000); + Assert.assertTrue("elapsed: " + elapsedMs, elapsedMs <= 12_000); error = true; } finally { RpcInvokeContext.removeContext(); + consumerConfig.unRefer(); } Assert.assertTrue(error);
436-449: Deadline vs timeout: stabilize timing; unrefer consumerUse nanoTime and tolerant bounds to avoid flakiness and unrefer the consumer.
- ServiceA serviceA = consumerConfig.refer(); - - long startTime = System.currentTimeMillis(); + ServiceA serviceA = consumerConfig.refer(); + long start = System.nanoTime(); boolean error = false; try { serviceA.processA("test-message"); Assert.fail("Should throw timeout exception due to deadline"); } catch (Exception e) { - long endTime = System.currentTimeMillis(); + long elapsedMs = (System.nanoTime() - start) / 1_000_000L; Assert.assertTrue(e instanceof SofaTimeOutException); - Assert.assertTrue(endTime - startTime <= 11000); // 应该按deadline超时 + // 9s deadline stricter than 25s timeout; allow tolerance + Assert.assertTrue("elapsed: " + elapsedMs, elapsedMs >= 7_000); + Assert.assertTrue("elapsed: " + elapsedMs, elapsedMs <= 12_000); error = true; - } + } finally { + consumerConfig.unRefer(); + } Assert.assertTrue(error);
🧹 Nitpick comments (2)
test/test-integration/src/test/java/com/alipay/sofa/rpc/test/deadline/AbstractDeadlineChainTest.java (2)
253-253: Sleep-based startup wait is brittle; prefer readiness checkFixed sleep of 1000ms can be flaky under load/CI variance. Prefer polling server readiness (e.g., awaiting server open port or performing a lightweight ping until success or timeout). If not readily available, increase wait and add retries.
I can add a small waitUntilAvailable(protocol, host, port, timeoutMs) helper here if you want.
50-57: Minor: processTime fields don’t need volatile in ServiceC; consider final for all threeprocessTime is set only in constructor; marking it final is clearer and avoids unnecessary volatility.
- private volatile int processTime; + private final int processTime;Apply the same to ServiceBImpl and keep ServiceAImpl as already final.
Also applies to: 72-79, 98-106
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
test/test-integration/src/test/java/com/alipay/sofa/rpc/test/deadline/AbstractDeadlineChainTest.java(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: sca
🔇 Additional comments (1)
test/test-integration/src/test/java/com/alipay/sofa/rpc/test/deadline/AbstractDeadlineChainTest.java (1)
267-310: Good: robust cleanup in finally for providers and intermediate consumersUnexporting providers and unreferencing the intermediate consumers in a finally block avoids resource leaks and port conflicts.
...t-integration/src/test/java/com/alipay/sofa/rpc/test/deadline/AbstractDeadlineChainTest.java
Outdated
Show resolved
Hide resolved
...t-integration/src/test/java/com/alipay/sofa/rpc/test/deadline/AbstractDeadlineChainTest.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
test/test-integration/src/test/java/com/alipay/sofa/rpc/test/deadline/AbstractDeadlineChainTest.java (2)
40-41: Good fix: AtomicBoolean ensures cross-thread visibilitySwitching from ThreadLocal to AtomicBoolean addresses the cross-thread observability issue for ServiceC start state.
170-171: Pass the bound ApplicationConfig into hooks to avoid divergenceAvoid constructing new ApplicationConfig instances for configureProvider/configureConsumer; pass the one already attached to the config object. This keeps hook behavior consistent with the actual app identity used by the configs.
Apply the following diffs:
- configureProvider(providerC, serverConfigC, new ApplicationConfig().setAppName("serviceC")); + configureProvider(providerC, serverConfigC, providerC.getApplication());- configureConsumer(consumerConfigC, protocol, urlC, new ApplicationConfig().setAppName("serviceB")); + configureConsumer(consumerConfigC, protocol, urlC, consumerConfigC.getApplication());- configureProvider(providerB, serverConfigB, new ApplicationConfig().setAppName("serviceB")); + configureProvider(providerB, serverConfigB, providerB.getApplication());- configureConsumer(consumerConfigB, protocol, urlB, new ApplicationConfig().setAppName("serviceA")); + configureConsumer(consumerConfigB, protocol, urlB, consumerConfigB.getApplication());- configureProvider(providerA, serverConfigA, new ApplicationConfig().setAppName("serviceA")); + configureProvider(providerA, serverConfigA, providerA.getApplication());- configureConsumer(consumerConfig, protocol, url, new ApplicationConfig().setAppName("client2")); + configureConsumer(consumerConfig, protocol, url, consumerConfig.getApplication());Also applies to: 188-191, 205-207, 223-226, 240-242, 305-307
🧹 Nitpick comments (2)
test/test-integration/src/test/java/com/alipay/sofa/rpc/test/deadline/AbstractDeadlineChainTest.java (2)
312-326: Unrefer the top-level consumer to prevent client-side resource leaksThe ConsumerConfig created in testDeadlineTimeout is not unreferenced, which can leak client connections/resources across tests.
Apply this diff to add a finally cleanup:
- isServiceCStarted.set(false); - ServiceA serviceA = consumerConfig.refer(); - - boolean error = false; - try { - serviceA.processA("test-message"); - Assert.fail("Should throw timeout exception"); - } catch (Exception e) { - Assert.assertTrue(e instanceof SofaTimeOutException); - Thread.sleep(9000); - Assert.assertFalse(isServiceCStarted.get()); - error = true; - } - Assert.assertTrue(error); + isServiceCStarted.set(false); + ServiceA serviceA = null; + boolean error = false; + try { + serviceA = consumerConfig.refer(); + serviceA.processA("test-message"); + Assert.fail("Should throw timeout exception"); + } catch (Exception e) { + Assert.assertTrue(e instanceof SofaTimeOutException); + Thread.sleep(9000); + Assert.assertFalse(isServiceCStarted.get()); + error = true; + } finally { + try { + consumerConfig.unRefer(); + } catch (Exception ex) { + System.err.println("Failed to unrefer consumerConfig for ServiceA: " + ex.getMessage()); + } + } + Assert.assertTrue(error);
244-244: Replace fixed startup sleep with readiness check to reduce flakinessThe fixed 1s delay may be insufficient on slow CI. Consider polling for server readiness or using server bootstrap readiness instead of a hard sleep.
- Option A (preferred): expose a small helper that verifies providerBootstrap/server is active before proceeding.
- Option B: increase the sleep or poll with a timeout (e.g., check directUrl connectivity).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
core-impl/client/src/main/java/com/alipay/sofa/rpc/client/AbstractCluster.java(3 hunks)core/api/src/main/java/com/alipay/sofa/rpc/common/RpcConstants.java(2 hunks)core/api/src/main/java/com/alipay/sofa/rpc/config/ConsumerConfig.java(1 hunks)core/api/src/main/java/com/alipay/sofa/rpc/context/RpcInvokeContext.java(3 hunks)core/api/src/main/java/com/alipay/sofa/rpc/filter/ProviderInvoker.java(3 hunks)test/test-integration/src/test/java/com/alipay/sofa/rpc/test/deadline/AbstractDeadlineChainTest.java(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- core/api/src/main/java/com/alipay/sofa/rpc/config/ConsumerConfig.java
🚧 Files skipped from review as they are similar to previous changes (4)
- core/api/src/main/java/com/alipay/sofa/rpc/common/RpcConstants.java
- core-impl/client/src/main/java/com/alipay/sofa/rpc/client/AbstractCluster.java
- core/api/src/main/java/com/alipay/sofa/rpc/filter/ProviderInvoker.java
- core/api/src/main/java/com/alipay/sofa/rpc/context/RpcInvokeContext.java
🧰 Additional context used
🧬 Code Graph Analysis (1)
test/test-integration/src/test/java/com/alipay/sofa/rpc/test/deadline/AbstractDeadlineChainTest.java (6)
core/api/src/main/java/com/alipay/sofa/rpc/config/ConsumerConfig.java (1)
ConsumerConfig(71-1044)core/api/src/main/java/com/alipay/sofa/rpc/config/ProviderConfig.java (1)
ProviderConfig(55-561)core/api/src/main/java/com/alipay/sofa/rpc/config/ServerConfig.java (1)
ServerConfig(66-909)core/api/src/main/java/com/alipay/sofa/rpc/context/RpcInvokeContext.java (1)
RpcInvokeContext(32-509)test/test-common/src/main/java/com/alipay/sofa/rpc/test/ActivelyDestroyTest.java (1)
ActivelyDestroyTest(33-48)core/api/src/main/java/com/alipay/sofa/rpc/common/RpcConstants.java (1)
RpcConstants(28-757)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build
- GitHub Check: sca
🔇 Additional comments (1)
test/test-integration/src/test/java/com/alipay/sofa/rpc/test/deadline/AbstractDeadlineChainTest.java (1)
63-73: Marking ServiceC start at method entry is correctSetting the flag before sleeping guarantees we can detect any unintended execution of ServiceC promptly.
2 participants
This is a Work In Progress pull request for issue #1498
By propagating the deadline value throughout the call chain, once the deadline expires, other pending tasks in the chain will not be executed.
Summary by CodeRabbit
New Features
Tests
Documentation