Copilot commented on code in PR #8062: URL: https://github.com/apache/incubator-seata/pull/8062#discussion_r3114545314
########## changes/en-us/2.x.md: ########## @@ -19,6 +19,7 @@ Add changes here for all PR submitted to the 2.x branch. <!-- Please add the `changes` to the following location(feature/bugfix/optimize/test) based on the type of PR --> ### feature: +- [[#8062](https://github.com/apache/incubator-seata/pull/???)] implement real TCC mode in benchmark CLI Review Comment: The changelog entry still contains a placeholder PR link (`pull/???`) and an inconsistent reference (`#8062`). Please replace with the actual PR number/link for this change before merging. ```suggestion ``` ########## test-suite/seata-benchmark-cli/src/main/java/org/apache/seata/benchmark/executor/TCCModeExecutor.java: ########## @@ -17,41 +17,128 @@ package org.apache.seata.benchmark.executor; import org.apache.seata.benchmark.config.BenchmarkConfig; +import org.apache.seata.benchmark.tcc.BenchmarkTccAction; +import org.apache.seata.benchmark.tcc.BenchmarkTccActionImpl; +import org.apache.seata.integration.tx.api.interceptor.InvocationWrapper; +import org.apache.seata.integration.tx.api.interceptor.handler.ProxyInvocationHandler; +import org.apache.seata.rm.tcc.interceptor.parser.TccActionInterceptorParser; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import java.lang.reflect.Method; +import java.lang.reflect.Proxy; + /** - * TCC mode transaction executor (simplified mock implementation) - * Note: For Phase 1 (MVP), TCC mode executes empty transactions similar to AT mode. - * Full TCC implementation with try/confirm/cancel will be added in future versions. + * TCC mode transaction executor. + * + * <p>Supports two sub-modes controlled by {@code --branches}: + * <ul> + * <li><b>Empty mode</b> ({@code branches == 0}): starts and commits an empty global + * transaction. Measures pure Seata protocol overhead with no branch registration.</li> + * <li><b>Real mode</b> ({@code branches > 0}): registers {@code branches} TCC branches + * per transaction via the {@link BenchmarkTccAction} proxy. On commit the TC invokes + * {@link BenchmarkTccAction#commit} for every registered branch; on rollback it invokes + * {@link BenchmarkTccAction#rollback}.</li> + * </ul> + * + * <p>No Spring container is required. A JDK dynamic proxy wraps + * {@link org.apache.seata.rm.tcc.interceptor.TccActionInterceptorHandler}, + * which is equivalent to what Spring AOP would produce at runtime. */ public class TCCModeExecutor extends AbstractTransactionExecutor { private static final Logger LOGGER = LoggerFactory.getLogger(TCCModeExecutor.class); + private BenchmarkTccAction actionProxy; + private BenchmarkTccActionImpl actionImpl; + public TCCModeExecutor(BenchmarkConfig config) { super(config); } + private boolean isRealMode() { + return config.getBranches() > 0; + } + @Override public void init() { - LOGGER.info("TCC mode executor initialized (simplified mock mode)"); + if (isRealMode()) { + initRealMode(); + } else { + LOGGER.info("TCC mode executor initialized (empty transaction mode)"); + } + } + + private void initRealMode() { + LOGGER.info("Initializing TCC mode executor (try/confirm/cancel)"); + + actionImpl = new BenchmarkTccActionImpl(); + + // TccActionInterceptorParser does two things: + // 1. Registers TCCResource with DefaultResourceManager (enables TC callbacks) + // 2. Returns a TccActionInterceptorHandler for proxy dispatch + TccActionInterceptorParser parser = new TccActionInterceptorParser(); + ProxyInvocationHandler handler; + try { + handler = parser.parserInterfaceToProxy(actionImpl, "benchmarkTccService"); + } catch (Exception e) { + throw new RuntimeException("Failed to initialize TCC proxy", e); + } Review Comment: `TccActionInterceptorParser#parserInterfaceToProxy(...)` can return `null` when no `@TwoPhaseBusinessAction` methods are found. If that happens, `handler.invoke(...)` below will throw a NPE with a confusing stack trace. Add an explicit null check after parsing and throw a clear exception (or log and fall back to empty mode). ```suggestion } if (handler == null) { throw new IllegalStateException("Failed to initialize TCC proxy: no @TwoPhaseBusinessAction method was found for " + BenchmarkTccAction.class.getName()); } ``` ########## changes/zh-cn/2.x.md: ########## @@ -20,6 +20,7 @@ ### feature: +- [[#8062](https://github.com/apache/incubator-seata/pull/???)] 为benchmark CLI实现真实TCC模式（try/confirm/cancel） Review Comment: The changelog entry still contains a placeholder PR link (`pull/???`) and an inconsistent reference (`#8062`). Please replace with the actual PR number/link for this change before merging. ```suggestion - [[#8062](https://github.com/apache/incubator-seata/pull/8062)] 为benchmark CLI实现真实TCC模式（try/confirm/cancel） ``` ########## test-suite/seata-benchmark-cli/src/main/java/org/apache/seata/benchmark/tcc/BenchmarkTccAction.java: ########## @@ -0,0 +1,43 @@ +/* + * 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 org.apache.seata.benchmark.tcc; + +import org.apache.seata.rm.tcc.api.BusinessActionContext; +import org.apache.seata.rm.tcc.api.LocalTCC; +import org.apache.seata.rm.tcc.api.TwoPhaseBusinessAction; + +/** + * Benchmark TCC action interface. + * + * <p>Used by {@link TCCModeExecutor} in real mode ({@code branches > 0}) to drive the TCC + * protocol path through the Seata interceptor. Each call to {@link #prepare} registers one + * TCC branch with the TC. On global commit the TC invokes {@link #commit}; on global rollback + * it invokes {@link #rollback}. Review Comment: The Javadoc references `{@link TCCModeExecutor}`, but that class is in a different package (`org.apache.seata.benchmark.executor`) and isn’t imported/fully-qualified here. This link won’t resolve during Javadoc generation (and can fail builds if doclint is strict). Use a fully-qualified link (or remove/replace the link). ```suggestion * <p>Used by {@link org.apache.seata.benchmark.executor.TCCModeExecutor} in real mode * ({@code branches > 0}) to drive the TCC protocol path through the Seata interceptor. * Each call to {@link #prepare} registers one TCC branch with the TC. On global commit * the TC invokes {@link #commit}; on global rollback it invokes {@link #rollback}. ``` -- 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]
