MatthewAden commented on code in PR #16352: URL: https://github.com/apache/dolphinscheduler/pull/16352#discussion_r1695277134
########## dolphinscheduler-registry/dolphinscheduler-registry-plugins/dolphinscheduler-registry-raft/src/main/java/org/apache/dolphinscheduler/plugin/registry/raft/client/IRaftRegisterClient.java: ########## @@ -0,0 +1,130 @@ +/* + * 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.dolphinscheduler.plugin.registry.raft.client; + +import org.apache.dolphinscheduler.registry.api.ConnectionListener; +import org.apache.dolphinscheduler.registry.api.SubscribeListener; + +import java.util.Collection; + +public interface IRaftRegisterClient extends AutoCloseable { + + /** + * Start the raft registry client. Once started, the client will connect to the raft registry server and then it can be used. + */ + void start(); + + /** + * Check the connectivity of the client. + * + * @return true if the client is connected, false otherwise + */ + boolean isConnectivity(); + + /** + * Subscribe to the raft registry connection state change event. + * + * @param connectionStateListener the listener to handle connection state changes + */ + void subscribeConnectionStateChange(ConnectionListener connectionStateListener); + + /** + * Subscribe to the register data change event. + * + * @param path the path to subscribe to + * @param listener the listener to handle data changes + */ + void subscribeRaftRegistryDataChange(String path, SubscribeListener listener); + + /** + * Get the raft register data by key. + * + * @param key the key of the register data + * @return the value associated with the key + */ + String getRegistryDataByKey(String key); + + /** + * Put the register data to the raft registry server. + * <p> + * If the key already exists, then update the value. If the key does not exist, then insert a new key-value pair. + * + * @param key the key of the register data + * @param value the value to be associated with the key + * @param deleteOnDisconnect if true, the data will be deleted when the client disconnects + */ + void putRegistryData(String key, String value, boolean deleteOnDisconnect); Review Comment: done ########## dolphinscheduler-registry/dolphinscheduler-registry-plugins/dolphinscheduler-registry-raft/src/main/java/org/apache/dolphinscheduler/plugin/registry/raft/RaftConnectionStateManager.java: ########## @@ -0,0 +1,141 @@ +/* + * 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.dolphinscheduler.plugin.registry.raft; + +import org.apache.dolphinscheduler.registry.api.ConnectionListener; +import org.apache.dolphinscheduler.registry.api.ConnectionState; + +import java.time.Duration; +import java.util.List; +import java.util.concurrent.CopyOnWriteArrayList; +import java.util.concurrent.Executors; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.TimeUnit; + +import lombok.extern.slf4j.Slf4j; + +import com.alipay.sofa.jraft.RouteTable; +import com.alipay.sofa.jraft.option.CliOptions; +import com.alipay.sofa.jraft.rpc.CliClientService; +import com.alipay.sofa.jraft.rpc.impl.cli.CliClientServiceImpl; +import com.google.common.util.concurrent.ThreadFactoryBuilder; + +@Slf4j +public class RaftConnectionStateManager implements IRaftConnectionStateManager { + + private static final String DEFAULT_REGION_ID = "--1"; + private ConnectionState currentConnectionState; + private static final Duration CONNECT_STATE_CHECK_INTERVENE = Duration.ofSeconds(2); + private final RaftRegistryProperties properties; + private final List<ConnectionListener> connectionListeners = new CopyOnWriteArrayList<>(); + private final ScheduledExecutorService scheduledExecutorService; + private final CliOptions cliOptions; + private final CliClientService cliClientService; + + public RaftConnectionStateManager(RaftRegistryProperties properties) { + this.properties = properties; + this.cliOptions = new CliOptions(); + this.cliOptions.setMaxRetry(3); + this.cliOptions.setTimeoutMs(5000); Review Comment: sorry to make you repeat the same thing twice. I have finished modifying all the magic numbers in the file. ########## dolphinscheduler-registry/dolphinscheduler-registry-plugins/dolphinscheduler-registry-raft/src/main/java/org/apache/dolphinscheduler/plugin/registry/raft/RaftLockManager.java: ########## @@ -0,0 +1,139 @@ +/* + * 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.dolphinscheduler.plugin.registry.raft; + +import org.apache.dolphinscheduler.common.thread.ThreadUtils; +import org.apache.dolphinscheduler.common.utils.NetUtils; +import org.apache.dolphinscheduler.common.utils.OSUtils; + +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.Executors; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.TimeUnit; + +import lombok.AllArgsConstructor; +import lombok.Builder; +import lombok.Data; +import lombok.NoArgsConstructor; + +import com.alipay.sofa.jraft.rhea.client.RheaKVStore; +import com.alipay.sofa.jraft.rhea.util.concurrent.DistributedLock; +import com.alipay.sofa.jraft.util.ExecutorServiceHelper; + +public class RaftLockManager implements IRaftLockManager { + + private final Map<String, LockEntry> distributedLockMap = new ConcurrentHashMap<>(); + private final RheaKVStore rheaKvStore; + private final RaftRegistryProperties raftRegistryProperties; + private static final ScheduledExecutorService WATCH_DOG = Executors.newSingleThreadScheduledExecutor(); Review Comment: This is an automatic lease renewal scheduler, and calling it watchdog might be more appropriate in this context. JRaft also refers to it as WatchDog. ########## dolphinscheduler-dist/release-docs/LICENSE: ########## @@ -572,11 +572,23 @@ The text of each license is also included at licenses/LICENSE-[project].txt. tea-rpc-util 0.1.3.jar https://github.com/aliyun/aliyun-openapi-java-sdk/blob/master/README.md#license Apache 2.0 tea-util 0.2.13.jar https://github.com/aliyun/aliyun-openapi-java-sdk/blob/master/README.md#license Apache 2.0 delight-nashorn-sandbox 0.3.2 https://github.com/javadelight/delight-nashorn-sandbox/blob/master/README.md#license Apache 2.0 - - - - - + jraft-core 1.3.14: https://mvnrepository.com/artifact/com.alipay.sofa/jraft-core/1.3.14 Apache 2.0 + jctools-core 2.1.1: https://mvnrepository.com/artifact/org.jctools/jctools-core/2.1.1 Apache 2.0 + commons-lang 2.6 https://mvnrepository.com/artifact/commons-lang/commons-lang/2.6 Apache 2.0 + hessian 3.3.6: https://mvnrepository.com/artifact/com.alipay.sofa/hessian/3.3.6 Apache 2.0 + metrics-core 4.2.11: https://mvnrepository.com/artifact/io.dropwizard.metrics/metrics-core/4.2.11 Apache 2.0 + affinity 3.1.7: https://mvnrepository.com/artifact/net.openhft/affinity/3.1.7, Apache 2.0 + disruptor 3.3.7: https://mvnrepository.com/artifact/com.lmax/disruptor/3.3.7, Apache 2.0 + commons-compress 1.21: https://mvnrepository.com/artifact/org.apache.commons/commons-compress/1.21 Apache 2.0 + bolt 1.6.4: https://mvnrepository.com/artifact/com.alipay.sofa/bolt/1.6.4, Apache 2.0 + rocksdbjni 8.8.1: https://mvnrepository.com/artifact/org.rocksdb/rocksdbjni/8.8.1, Apache 2.0 + jraft-rheakv-core 1.3.14: https://mvnrepository.com/artifact/com.alipay.sofa/jraft-rheakv-core/1.3.14, Apache 2.0 + sofa-common-tools 1.0.12: https://mvnrepository.com/artifact/com.alipay.sofa.common/sofa-common-tools/1.0.12 Apache 2.0 + annotations 12.0: https://mvnrepository.com/artifact/com.intellij/annotations/12.0 Apache 2.0 + protostuff-core 1.7.2: https://mvnrepository.com/artifact/io.protostuff/protostuff-core/1.7.2 Apache 2.0 + protostuff-api 1.7.2: https://mvnrepository.com/artifact/io.protostuff/protostuff-api/1.7.2 Apache 2.0 + protostuff-runtime 1.7.2: https://mvnrepository.com/artifact/io.protostuff/protostuff-runtime/1.7.2 Apache 2.0 + protostuff-collectionschema 1.7.2: https://mvnrepository.com/artifact/io.protostuff/protostuff-collectionschema/1.7.2 Apache 2.0 Review Comment: We introduced this direct dependency. <dependency> <groupId>com.alipay.sofa</groupId> <artifactId>jraft-rheakv-core</artifactId> <version>${jraft.version}</version> </dependency> </dependencies> These are all transitive dependencies. I'm not sure if these need to be written in the license. ########## dolphinscheduler-registry/dolphinscheduler-registry-plugins/dolphinscheduler-registry-raft/src/main/java/org/apache/dolphinscheduler/plugin/registry/raft/RaftConnectionStateManager.java: ########## @@ -0,0 +1,141 @@ +/* + * 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.dolphinscheduler.plugin.registry.raft; + +import org.apache.dolphinscheduler.registry.api.ConnectionListener; +import org.apache.dolphinscheduler.registry.api.ConnectionState; + +import java.time.Duration; +import java.util.List; +import java.util.concurrent.CopyOnWriteArrayList; +import java.util.concurrent.Executors; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.TimeUnit; + +import lombok.extern.slf4j.Slf4j; + +import com.alipay.sofa.jraft.RouteTable; +import com.alipay.sofa.jraft.option.CliOptions; +import com.alipay.sofa.jraft.rpc.CliClientService; +import com.alipay.sofa.jraft.rpc.impl.cli.CliClientServiceImpl; +import com.google.common.util.concurrent.ThreadFactoryBuilder; + +@Slf4j +public class RaftConnectionStateManager implements IRaftConnectionStateManager { + + private static final String DEFAULT_REGION_ID = "--1"; + private ConnectionState currentConnectionState; + private static final Duration CONNECT_STATE_CHECK_INTERVENE = Duration.ofSeconds(2); + private final RaftRegistryProperties properties; + private final List<ConnectionListener> connectionListeners = new CopyOnWriteArrayList<>(); + private final ScheduledExecutorService scheduledExecutorService; + private final CliOptions cliOptions; + private final CliClientService cliClientService; + + public RaftConnectionStateManager(RaftRegistryProperties properties) { + this.properties = properties; + this.cliOptions = new CliOptions(); + this.cliOptions.setMaxRetry(3); + this.cliOptions.setTimeoutMs(5000); + this.cliClientService = new CliClientServiceImpl(); + this.scheduledExecutorService = Executors.newScheduledThreadPool( + 1, + new ThreadFactoryBuilder().setNameFormat("ConnectionStateRefreshThread").setDaemon(true).build()); + } + + public void start() { + cliClientService.init(cliOptions); + scheduledExecutorService.scheduleWithFixedDelay( + new ConnectionStateRefreshTask(connectionListeners), + CONNECT_STATE_CHECK_INTERVENE.toMillis(), + CONNECT_STATE_CHECK_INTERVENE.toMillis(), + TimeUnit.MILLISECONDS); + } + + public void addConnectionListener(ConnectionListener listener) { Review Comment: updated ########## dolphinscheduler-registry/dolphinscheduler-registry-plugins/dolphinscheduler-registry-raft/src/main/java/org/apache/dolphinscheduler/plugin/registry/raft/RaftLockManager.java: ########## @@ -0,0 +1,139 @@ +/* + * 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.dolphinscheduler.plugin.registry.raft; + +import org.apache.dolphinscheduler.common.thread.ThreadUtils; +import org.apache.dolphinscheduler.common.utils.NetUtils; +import org.apache.dolphinscheduler.common.utils.OSUtils; + +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.Executors; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.TimeUnit; + +import lombok.AllArgsConstructor; +import lombok.Builder; +import lombok.Data; +import lombok.NoArgsConstructor; + +import com.alipay.sofa.jraft.rhea.client.RheaKVStore; +import com.alipay.sofa.jraft.rhea.util.concurrent.DistributedLock; +import com.alipay.sofa.jraft.util.ExecutorServiceHelper; + +public class RaftLockManager implements IRaftLockManager { + + private final Map<String, LockEntry> distributedLockMap = new ConcurrentHashMap<>(); + private final RheaKVStore rheaKvStore; + private final RaftRegistryProperties raftRegistryProperties; + private static final ScheduledExecutorService WATCH_DOG = Executors.newSingleThreadScheduledExecutor(); + private static final String LOCK_OWNER_PREFIX = NetUtils.getHost() + "_" + OSUtils.getProcessID() + "_"; + + public RaftLockManager(RheaKVStore rheaKVStore, RaftRegistryProperties raftRegistryProperties) { + this.rheaKvStore = rheaKVStore; + this.raftRegistryProperties = raftRegistryProperties; + } + + @Override + public boolean acquireLock(String lockKey) { + final String lockOwner = getLockOwnerPrefix(); + if (isThreadReentrant(lockKey, lockOwner)) { + return true; + } + + final DistributedLock<byte[]> distributedLock = rheaKvStore.getDistributedLock(lockKey, + raftRegistryProperties.getDistributedLockTimeout().toMillis(), TimeUnit.MILLISECONDS, WATCH_DOG); + + while (true) { + if (distributedLock.tryLock()) { + distributedLockMap.put(lockKey, LockEntry.builder().distributedLock(distributedLock) + .lockOwner(lockOwner) + .build()); + return true; + } else { + // fail to acquire lock + ThreadUtils.sleep(raftRegistryProperties.getDistributedLockRetryInterval().toMillis()); + } + } + } + + @Override + public boolean acquireLock(String lockKey, long timeout) { + final String lockOwner = getLockOwnerPrefix(); + if (isThreadReentrant(lockKey, lockOwner)) { + return true; + } + final long endTime = System.currentTimeMillis() + timeout; + final DistributedLock<byte[]> distributedLock = rheaKvStore.getDistributedLock(lockKey, + raftRegistryProperties.getDistributedLockTimeout().toMillis(), TimeUnit.MILLISECONDS, WATCH_DOG); + + while (System.currentTimeMillis() < endTime) { + if (distributedLock.tryLock()) { + distributedLockMap.put(lockKey, LockEntry.builder().distributedLock(distributedLock) + .lockOwner(lockOwner) + .build()); + return true; + } else { + // fail to acquire lock + ThreadUtils.sleep(raftRegistryProperties.getDistributedLockRetryInterval().toMillis()); + } + } + + return false; + } + + private boolean isThreadReentrant(String lockKey, String lockOwner) { + final LockEntry lockEntry = distributedLockMap.get(lockKey); + return lockEntry != null && lockOwner.equals(lockEntry.getLockOwner()); + } + + @Override + public boolean releaseLock(String lockKey) { + final String lockOwner = getLockOwnerPrefix(); + final LockEntry lockEntry = distributedLockMap.get(lockKey); + if (lockEntry == null || !lockOwner.equals(lockEntry.getLockOwner())) { + return false; + } + + final DistributedLock<byte[]> distributedLock = distributedLockMap.get(lockKey).getDistributedLock(); + if (distributedLock != null) { + distributedLock.unlock(); + } + distributedLockMap.remove(lockKey); + return true; + } + + public static String getLockOwnerPrefix() { + return LOCK_OWNER_PREFIX + Thread.currentThread().getName(); + } + + @Data + @AllArgsConstructor + @NoArgsConstructor + @Builder + public static class LockEntry { + + private DistributedLock<byte[]> distributedLock; + private String lockOwner; + } Review Comment: done ########## dolphinscheduler-registry/dolphinscheduler-registry-plugins/dolphinscheduler-registry-raft/src/main/java/org/apache/dolphinscheduler/plugin/registry/raft/RaftConnectionStateManager.java: ########## @@ -0,0 +1,141 @@ +/* + * 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.dolphinscheduler.plugin.registry.raft; + +import org.apache.dolphinscheduler.registry.api.ConnectionListener; +import org.apache.dolphinscheduler.registry.api.ConnectionState; + +import java.time.Duration; +import java.util.List; +import java.util.concurrent.CopyOnWriteArrayList; +import java.util.concurrent.Executors; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.TimeUnit; + +import lombok.extern.slf4j.Slf4j; + +import com.alipay.sofa.jraft.RouteTable; +import com.alipay.sofa.jraft.option.CliOptions; +import com.alipay.sofa.jraft.rpc.CliClientService; +import com.alipay.sofa.jraft.rpc.impl.cli.CliClientServiceImpl; +import com.google.common.util.concurrent.ThreadFactoryBuilder; + +@Slf4j +public class RaftConnectionStateManager implements IRaftConnectionStateManager { + + private static final String DEFAULT_REGION_ID = "--1"; + private ConnectionState currentConnectionState; + private static final Duration CONNECT_STATE_CHECK_INTERVENE = Duration.ofSeconds(2); + private final RaftRegistryProperties properties; + private final List<ConnectionListener> connectionListeners = new CopyOnWriteArrayList<>(); + private final ScheduledExecutorService scheduledExecutorService; + private final CliOptions cliOptions; + private final CliClientService cliClientService; + + public RaftConnectionStateManager(RaftRegistryProperties properties) { + this.properties = properties; + this.cliOptions = new CliOptions(); + this.cliOptions.setMaxRetry(3); + this.cliOptions.setTimeoutMs(5000); + this.cliClientService = new CliClientServiceImpl(); + this.scheduledExecutorService = Executors.newScheduledThreadPool( + 1, + new ThreadFactoryBuilder().setNameFormat("ConnectionStateRefreshThread").setDaemon(true).build()); + } + @Override + public void start() { + cliClientService.init(cliOptions); + scheduledExecutorService.scheduleWithFixedDelay( + new ConnectionStateRefreshTask(connectionListeners), + CONNECT_STATE_CHECK_INTERVENE.toMillis(), + CONNECT_STATE_CHECK_INTERVENE.toMillis(), + TimeUnit.MILLISECONDS); + } + @Override + public void addConnectionListener(ConnectionListener listener) { + connectionListeners.add(listener); + } + + @Override + public ConnectionState getConnectionState() { + return currentConnectionState; + } + + class ConnectionStateRefreshTask implements Runnable { + + private final List<ConnectionListener> connectionListeners; + ConnectionStateRefreshTask(List<ConnectionListener> connectionListeners) { + this.connectionListeners = connectionListeners; + } + + @Override + public void run() { + try { + ConnectionState newConnectionState = getCurrentConnectionState(); + if (newConnectionState == currentConnectionState) { + // no state change + return; + } + if (newConnectionState == ConnectionState.DISCONNECTED + && currentConnectionState == ConnectionState.CONNECTED) { + currentConnectionState = ConnectionState.DISCONNECTED; + triggerListeners(ConnectionState.DISCONNECTED); + } else if (newConnectionState == ConnectionState.CONNECTED + && currentConnectionState == ConnectionState.DISCONNECTED) { + currentConnectionState = ConnectionState.CONNECTED; + triggerListeners(ConnectionState.RECONNECTED); + } else if (currentConnectionState == null) { + currentConnectionState = newConnectionState; + triggerListeners(currentConnectionState); + } + } catch (Exception ex) { + log.error("raft registry connection state check failed", ex); + currentConnectionState = ConnectionState.DISCONNECTED; + triggerListeners(ConnectionState.DISCONNECTED); + } + } + + private ConnectionState getCurrentConnectionState() { + try { + String groupId = properties.getClusterName() + DEFAULT_REGION_ID; + if (RouteTable.getInstance().refreshLeader(cliClientService, groupId, 2000).isOk()) { Review Comment: done ########## dolphinscheduler-registry/dolphinscheduler-registry-plugins/dolphinscheduler-registry-raft/src/main/java/org/apache/dolphinscheduler/plugin/registry/raft/RaftRegistryProperties.java: ########## @@ -0,0 +1,51 @@ +/* + * 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.dolphinscheduler.plugin.registry.raft; + +import java.time.Duration; + +import lombok.Data; + +import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; +import org.springframework.boot.context.properties.ConfigurationProperties; +import org.springframework.context.annotation.Configuration; + +@Data +@Configuration +@ConditionalOnProperty(prefix = "registry", name = "type", havingValue = "raft") +@ConfigurationProperties(prefix = "registry") +public class RaftRegistryProperties { + + private String clusterName; + private String serverAddressList; + private String serverAddress; + private int serverPort; + private String logStorageDir; + private Duration distributedLockTimeout = Duration.ofSeconds(3); + private Duration distributedLockRetryInterval = Duration.ofSeconds(5); + private String module = "master"; + private Duration listenerCheckInterval = Duration.ofSeconds(3); + private int cliMaxRetries = 3; + private Duration cliTimeout = Duration.ofSeconds(5); + private Duration refreshLeaderTimeout = Duration.ofSeconds(2); + private Duration connectStateCheckInterval = Duration.ofSeconds(2); + private Duration heartBeatTimeOut = Duration.ofSeconds(20); + private int subscribeListenerThreadPoolSize = 1; + private int connectionListenerThreadPoolSize = 1; Review Comment: The following configurations are the JRaft component use. private String clusterName; private String serverAddressList; private String serverAddress; private int serverPort; private String logStorageDir; private Duration distributedLockTimeout = Duration.ofSeconds(3); private int cliMaxRetries = 3; private Duration cliTimeout = Duration.ofSeconds(5); private Duration refreshLeaderTimeout = Duration.ofSeconds(2); The following are some configurations we used ourselves. The previous CodeReview said not to use so many image numbers, so I put all of them here in the configuration. private Duration distributedLockRetryInterval = Duration.ofSeconds(5); private String module = "master"; private Duration heartBeatTimeOut = Duration.ofSeconds(20); private Duration listenerCheckInterval = Duration.ofSeconds(3); private Duration connectStateCheckInterval = Duration.ofSeconds(2); private int subscribeListenerThreadPoolSize = 1; private int connectionListenerThreadPoolSize = 1; What should be done to make it better? ########## dolphinscheduler-registry/dolphinscheduler-registry-plugins/dolphinscheduler-registry-raft/src/main/java/org/apache/dolphinscheduler/plugin/registry/raft/RaftRegistry.java: ########## @@ -0,0 +1,142 @@ +/* + * 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.dolphinscheduler.plugin.registry.raft; + +import static com.google.common.base.Preconditions.checkNotNull; + +import org.apache.dolphinscheduler.common.thread.ThreadUtils; +import org.apache.dolphinscheduler.plugin.registry.raft.client.IRaftRegisterClient; +import org.apache.dolphinscheduler.plugin.registry.raft.client.RaftRegisterClient; +import org.apache.dolphinscheduler.registry.api.ConnectionListener; +import org.apache.dolphinscheduler.registry.api.Registry; +import org.apache.dolphinscheduler.registry.api.RegistryException; +import org.apache.dolphinscheduler.registry.api.SubscribeListener; + +import java.time.Duration; +import java.util.Collection; + +import lombok.NonNull; +import lombok.extern.slf4j.Slf4j; + +@Slf4j +public class RaftRegistry implements Registry { + + private static final long RECONNECT_WAIT_TIME_MS = 50L; Review Comment: it has been defined as a constant and then used. ########## dolphinscheduler-registry/dolphinscheduler-registry-plugins/dolphinscheduler-registry-raft/src/main/java/org/apache/dolphinscheduler/plugin/registry/raft/RaftRegistryAutoConfiguration.java: ########## @@ -0,0 +1,72 @@ +/* + * 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.dolphinscheduler.plugin.registry.raft; + +import org.apache.dolphinscheduler.plugin.registry.raft.server.RaftRegistryServer; + +import lombok.extern.slf4j.Slf4j; + +import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; +import org.springframework.boot.context.properties.EnableConfigurationProperties; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.context.annotation.DependsOn; + +@Slf4j +@Configuration(proxyBeanMethods = false) +@ConditionalOnProperty(prefix = "registry", name = "type", havingValue = "raft") +@EnableConfigurationProperties(RaftRegistryProperties.class) +public class RaftRegistryAutoConfiguration { + + public RaftRegistryAutoConfiguration() { + log.info("Load RaftRegistryAutoConfiguration"); + } + + @Bean + @ConditionalOnProperty(prefix = "registry", name = "module", havingValue = "master") + public RaftRegistryServer raftRegistryServer(RaftRegistryProperties raftRegistryProperties) { + RaftRegistryServer raftRegistryServer = new RaftRegistryServer(raftRegistryProperties); + raftRegistryServer.start(); + return raftRegistryServer; + } + + @Bean + @DependsOn("raftRegistryServer") + @ConditionalOnProperty(prefix = "registry", name = "module", havingValue = "master") + public RaftRegistry masterRaftRegistryClient(RaftRegistryProperties raftRegistryProperties) { + RaftRegistry raftRegistry = new RaftRegistry(raftRegistryProperties); + raftRegistry.start(); + return raftRegistry; + } + + @Bean + @ConditionalOnProperty(prefix = "registry", name = "module", havingValue = "worker") + public RaftRegistry workerRaftRegistryClient(RaftRegistryProperties raftRegistryProperties) { + RaftRegistry raftRegistry = new RaftRegistry(raftRegistryProperties); + raftRegistry.start(); + return raftRegistry; + } + + @Bean + @ConditionalOnProperty(prefix = "registry", name = "module", havingValue = "api") + public RaftRegistry apiRaftRegistryClient(RaftRegistryProperties raftRegistryProperties) { + RaftRegistry raftRegistry = new RaftRegistry(raftRegistryProperties); + raftRegistry.start(); + return raftRegistry; + } Review Comment: I want to achieve is that the master start the client and server of raft, while other modules only need to start the client of raft. Is there a better way to achieve this? ########## dolphinscheduler-registry/dolphinscheduler-registry-plugins/dolphinscheduler-registry-raft/src/main/java/org/apache/dolphinscheduler/plugin/registry/raft/client/RaftRegistryClient.java: ########## @@ -0,0 +1,221 @@ +/* + * 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.dolphinscheduler.plugin.registry.raft.client; + +import static com.alipay.sofa.jraft.util.BytesUtil.readUtf8; +import static com.alipay.sofa.jraft.util.BytesUtil.writeUtf8; + +import org.apache.dolphinscheduler.common.constants.Constants; +import org.apache.dolphinscheduler.plugin.registry.raft.RaftRegistryProperties; +import org.apache.dolphinscheduler.plugin.registry.raft.manage.IRaftConnectionStateManager; +import org.apache.dolphinscheduler.plugin.registry.raft.manage.IRaftLockManager; +import org.apache.dolphinscheduler.plugin.registry.raft.manage.IRaftSubscribeDataManager; +import org.apache.dolphinscheduler.plugin.registry.raft.manage.RaftConnectionStateManager; +import org.apache.dolphinscheduler.plugin.registry.raft.manage.RaftLockManager; +import org.apache.dolphinscheduler.plugin.registry.raft.manage.RaftSubscribeDataManager; +import org.apache.dolphinscheduler.plugin.registry.raft.model.NodeType; +import org.apache.dolphinscheduler.registry.api.ConnectionListener; +import org.apache.dolphinscheduler.registry.api.ConnectionState; +import org.apache.dolphinscheduler.registry.api.RegistryException; +import org.apache.dolphinscheduler.registry.api.SubscribeListener; +import org.apache.dolphinscheduler.registry.api.enums.RegistryNodeType; + +import java.util.ArrayList; +import java.util.Collection; +import java.util.List; + +import lombok.extern.slf4j.Slf4j; + +import com.alipay.sofa.jraft.rhea.client.DefaultRheaKVStore; +import com.alipay.sofa.jraft.rhea.client.RheaKVStore; +import com.alipay.sofa.jraft.rhea.options.PlacementDriverOptions; +import com.alipay.sofa.jraft.rhea.options.RegionRouteTableOptions; +import com.alipay.sofa.jraft.rhea.options.RheaKVStoreOptions; +import com.alipay.sofa.jraft.rhea.options.configured.MultiRegionRouteTableOptionsConfigured; +import com.alipay.sofa.jraft.rhea.options.configured.PlacementDriverOptionsConfigured; +import com.alipay.sofa.jraft.rhea.options.configured.RheaKVStoreOptionsConfigured; +import com.alipay.sofa.jraft.rhea.storage.KVEntry; + +@Slf4j +public class RaftRegistryClient implements IRaftRegistryClient { + + private final RheaKVStore rheaKvStore; + private final RaftRegistryProperties raftRegistryProperties; + private final IRaftConnectionStateManager raftConnectionStateManager; + private final IRaftSubscribeDataManager raftSubscribeDataManager; + private final IRaftLockManager raftLockManager; + private volatile boolean started; + private static final String MASTER_MODULE = "master"; + public RaftRegistryClient(RaftRegistryProperties raftRegistryProperties) { + this.raftRegistryProperties = raftRegistryProperties; + this.rheaKvStore = new DefaultRheaKVStore(); + this.raftConnectionStateManager = new RaftConnectionStateManager(raftRegistryProperties); + this.raftSubscribeDataManager = new RaftSubscribeDataManager(raftRegistryProperties, rheaKvStore); + this.raftLockManager = new RaftLockManager(rheaKvStore, raftRegistryProperties); + + initRheakv(); + } + + private void initRheakv() { + final List<RegionRouteTableOptions> regionRouteTableOptionsList = MultiRegionRouteTableOptionsConfigured + .newConfigured() + .withInitialServerList(-1L /* default id */, raftRegistryProperties.getServerAddressList()) + .config(); + final PlacementDriverOptions pdOpts = PlacementDriverOptionsConfigured.newConfigured() + .withFake(true) + .withRegionRouteTableOptionsList(regionRouteTableOptionsList) + .config(); + final RheaKVStoreOptions opts = RheaKVStoreOptionsConfigured.newConfigured() // + .withClusterName(raftRegistryProperties.getClusterName()) // + .withPlacementDriverOptions(pdOpts) // + .config(); + this.rheaKvStore.init(opts); + } + + @Override + public void start() { + if (this.started) { + log.info("RaftRegistryClient is already started"); + return; + } + log.info("starting raft client registry..."); + if (raftRegistryProperties.getModule().equals(MASTER_MODULE)) { + raftSubscribeDataManager.start(); + } Review Comment: In my understanding, only the master listens for changes in the registered instances -- 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]
