You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@rocketmq.apache.org by "yyqdbngt (via GitHub)" <gi...@apache.org> on 2026/08/14 07:30:21 UTC
[PR] fix(tencent): coordinate client creation and invalidation [rocketmq-dashboard]
yyqdbngt opened a new pull request, #2196:
URL: https://github.com/apache/rocketmq-dashboard/pull/2196
## Summary
- coordinate Tencent SDK client cache misses with credential invalidation
- keep existing cache hits on the lock-free path
- add a deterministic concurrency regression test for an in-flight client creation
## Root cause
A computeIfAbsent mapping is not available to the invalidation key scan until creation completes. Invalidation could therefore return while an old-credential client was still being built, and that client would then be installed in the cache afterward.
Closes #2195
## Validation
- mvn -Dtest=TencentClientFactoryTest test (4 tests)
- mvn -DskipTests checkstyle:check (0 violations)
- git diff --check
- verified both changed paths against all open PRs targeting ocketmq-studio
- audited full PR history for the configured contributor accounts
--
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: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(tencent): coordinate client creation and invalidation [rocketmq-dashboard]
Posted by "lizhimins (via GitHub)" <gi...@apache.org>.
lizhimins merged PR #2196:
URL: https://github.com/apache/rocketmq-dashboard/pull/2196
--
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: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(tencent): coordinate client creation and invalidation [rocketmq-dashboard]
Posted by "RockteMQ-AI (via GitHub)" <gi...@apache.org>.
RockteMQ-AI commented on code in PR #2196:
URL: https://github.com/apache/rocketmq-dashboard/pull/2196#discussion_r3781963377
##########
server/src/test/java/org/apache/rocketmq/studio/provider/tencent/TencentClientFactoryTest.java:
##########
@@ -45,4 +53,62 @@ void endpointForShouldUseRegionalEndpointForFinancialRegionsTest() {
assertThat(TencentClientFactory.endpointFor("ap-shanghai-fsi"))
.isEqualTo("trocket.ap-shanghai-fsi.tencentcloudapi.com");
}
+
+ @Test
+ void invalidateCredentialShouldRemoveAnInFlightClientCreation() throws Exception {
+ BlockingClientFactory factory = new BlockingClientFactory();
+ FutureTask<TrocketClient> firstClientTask = new FutureTask<>(
+ () -> factory.client("credential-a", "ap-shanghai"));
+ Thread creationThread = new Thread(firstClientTask, "tencent-client-creation-test");
+ creationThread.start();
+ assertThat(factory.creationStarted.await(5, TimeUnit.SECONDS)).isTrue();
+
+ Thread invalidationThread = new Thread(
+ () -> factory.invalidateCredential("credential-a"),
+ "tencent-client-invalidation-test");
+ invalidationThread.start();
+ awaitBlockedOrTerminated(invalidationThread);
+
+ factory.allowCreation.countDown();
+ TrocketClient firstClient = firstClientTask.get(5, TimeUnit.SECONDS);
+ invalidationThread.join(TimeUnit.SECONDS.toMillis(5));
+
+ assertThat(invalidationThread.isAlive()).isFalse();
+ assertThat(factory.client("credential-a", "ap-shanghai")).isNotSameAs(firstClient);
+ assertThat(factory.creationCount).hasValue(2);
+ }
+
+ private static void awaitBlockedOrTerminated(Thread thread) {
+ long deadline = System.nanoTime() + TimeUnit.SECONDS.toNanos(5);
+ while (thread.getState() != Thread.State.BLOCKED && thread.isAlive()
Review Comment:
awaitBlockedOrTerminated relies on Thread.getState() and spin-waiting to observe monitor contention. While deterministic in this controlled test, assertions based on thread states can be brittle under different JVM schedulers.
##########
server/src/main/java/org/apache/rocketmq/studio/provider/tencent/TencentClientFactory.java:
##########
@@ -48,10 +48,16 @@ public class TencentClientFactory {
public TrocketClient client(String credentialId, String region) {
String key = cacheKey(credentialId, region);
- return clients.computeIfAbsent(key, ignored -> createClient(credentialId, region));
+ TrocketClient cached = clients.get(key);
+ if (cached != null) {
+ return cached;
+ }
+ synchronized (this) {
Review Comment:
Cache misses now synchronize on the factory instance, serializing all client creation across every credential/region. If this path is heavily contended, consider a finer-grained lock (e.g., per-key or a read/write lock) instead of a global monitor.
##########
server/src/main/java/org/apache/rocketmq/studio/provider/tencent/TencentClientFactory.java:
##########
@@ -48,10 +48,16 @@ public class TencentClientFactory {
public TrocketClient client(String credentialId, String region) {
String key = cacheKey(credentialId, region);
Review Comment:
The lock-free fast-path get can return a cached client while invalidateCredential is concurrently removing it, leaving a brief stale-read window. This appears intentional to preserve hit performance; confirm it is acceptable for the credential-rotation semantics.
--
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: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org