You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@shardingsphere.apache.org by "Pace2Car (via GitHub)" <gi...@apache.org> on 2023/03/03 09:11:43 UTC

[GitHub] [shardingsphere] Pace2Car opened a new pull request, #24441: Add DistSQL LOCK CLUSTER

Pace2Car opened a new pull request, #24441:
URL: https://github.com/apache/shardingsphere/pull/24441

   For #24338.
   
   Changes proposed in this pull request:
     - Add DistSQL LOCK CLUSTER,support stop write, stop read-write lock 
     - Add related test case
     - Update document
   
   ---
   
   Before committing this PR, I'm sure that I have checked the following options:
   - [x] My code follows the [code of conduct](https://shardingsphere.apache.org/community/en/involved/conduct/code/) of this project.
   - [x] I have self-reviewed the commit code.
   - [ ] I have (or in comment I request) added corresponding labels for the pull request.
   - [x] I have passed maven check locally : `./mvnw clean install -B -T1C -Dmaven.javadoc.skip -Dmaven.jacoco.skip -e`.
   - [ ] I have made corresponding changes to the documentation.
   - [x] I have added corresponding unit tests for my changes.
   


-- 
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: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [shardingsphere] RaigorJiang commented on a diff in pull request #24441: Add DistSQL LOCK CLUSTER

Posted by "RaigorJiang (via GitHub)" <gi...@apache.org>.
RaigorJiang commented on code in PR #24441:
URL: https://github.com/apache/shardingsphere/pull/24441#discussion_r1125984928


##########
proxy/backend/core/src/main/java/org/apache/shardingsphere/proxy/backend/state/impl/ReadOnlyProxyState.java:
##########
@@ -0,0 +1,47 @@
+/*
+ * 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.shardingsphere.proxy.backend.state.impl;
+
+import org.apache.shardingsphere.proxy.backend.exception.ReadOnlyException;
+import org.apache.shardingsphere.proxy.backend.state.spi.ProxyClusterState;
+import org.apache.shardingsphere.sql.parser.sql.common.statement.SQLStatement;
+import org.apache.shardingsphere.sql.parser.sql.common.statement.dal.ShowStatement;
+import org.apache.shardingsphere.sql.parser.sql.common.statement.dml.SelectStatement;
+import org.apache.shardingsphere.sql.parser.sql.dialect.statement.mysql.dal.MySQLShowDatabasesStatement;
+
+/**
+ * ReadOnly proxy state.
+ */
+public final class ReadOnlyProxyState implements ProxyClusterState {
+    
+    @Override
+    public void check(final SQLStatement sqlStatement) {
+        if (isWriteStatement(sqlStatement)) {
+            throw new ReadOnlyException();
+        }
+    }
+    
+    private boolean isWriteStatement(final SQLStatement sqlStatement) {
+        return !(sqlStatement instanceof SelectStatement) && !(sqlStatement instanceof ShowStatement) && !(sqlStatement instanceof MySQLShowDatabasesStatement);

Review Comment:
   @Pace2Car What about DistSQLStatement?



-- 
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: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [shardingsphere] codecov-commenter commented on pull request #24441: Add DistSQL LOCK CLUSTER

Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #24441:
URL: https://github.com/apache/shardingsphere/pull/24441#issuecomment-1453248106

   # [Codecov](https://codecov.io/gh/apache/shardingsphere/pull/24441?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#24441](https://codecov.io/gh/apache/shardingsphere/pull/24441?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (94d8c16) into [master](https://codecov.io/gh/apache/shardingsphere/commit/bc11038fbce15aa56f5026c5e0b5c406599c9103?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (bc11038) will **decrease** coverage by `0.01%`.
   > The diff coverage is `49.59%`.
   
   > :exclamation: Current head 94d8c16 differs from pull request most recent head 58487a4. Consider uploading reports for the commit 58487a4 to get more accurate results
   
   ```diff
   @@             Coverage Diff              @@
   ##             master   #24441      +/-   ##
   ============================================
   - Coverage     49.92%   49.92%   -0.01%     
   - Complexity     1584     1588       +4     
   ============================================
     Files          3257     3269      +12     
     Lines         53576    53682     +106     
     Branches       9868     9883      +15     
   ============================================
   + Hits          26749    26800      +51     
   - Misses        24430    24479      +49     
   - Partials       2397     2403       +6     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/shardingsphere/pull/24441?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...ser/core/kernel/KernelDistSQLStatementVisitor.java](https://codecov.io/gh/apache/shardingsphere/pull/24441?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGlzdHNxbC9wYXJzZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL2Rpc3RzcWwvcGFyc2VyL2NvcmUva2VybmVsL0tlcm5lbERpc3RTUUxTdGF0ZW1lbnRWaXNpdG9yLmphdmE=) | `5.26% <0.00%> (-0.06%)` | :arrow_down: |
   | [...phere/infra/state/cluster/ClusterStateContext.java](https://codecov.io/gh/apache/shardingsphere/pull/24441?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-aW5mcmEvY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9pbmZyYS9zdGF0ZS9jbHVzdGVyL0NsdXN0ZXJTdGF0ZUNvbnRleHQuamF2YQ==) | `0.00% <0.00%> (ø)` | |
   | [...ngsphere/infra/state/cluster/ClusterStateType.java](https://codecov.io/gh/apache/shardingsphere/pull/24441?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-aW5mcmEvY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9pbmZyYS9zdGF0ZS9jbHVzdGVyL0NsdXN0ZXJTdGF0ZVR5cGUuamF2YQ==) | `0.00% <0.00%> (ø)` | |
   | [...data/pipeline/cdc/client/util/AnyValueConvert.java](https://codecov.io/gh/apache/shardingsphere/pull/24441?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a2VybmVsL2RhdGEtcGlwZWxpbmUvY2RjL2NsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvZGF0YS9waXBlbGluZS9jZGMvY2xpZW50L3V0aWwvQW55VmFsdWVDb252ZXJ0LmphdmE=) | `0.00% <0.00%> (ø)` | |
   | [...shardingsphere/mode/lock/GlobalLockDefinition.java](https://codecov.io/gh/apache/shardingsphere/pull/24441?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-bW9kZS9jb3JlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9tb2RlL2xvY2svR2xvYmFsTG9ja0RlZmluaXRpb24uamF2YQ==) | `100.00% <ø> (ø)` | |
   | [...sphere/mode/metadata/persist/node/ComputeNode.java](https://codecov.io/gh/apache/shardingsphere/pull/24441?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-bW9kZS9jb3JlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9tb2RlL21ldGFkYXRhL3BlcnNpc3Qvbm9kZS9Db21wdXRlTm9kZS5qYXZh) | `64.70% <0.00%> (-4.05%)` | :arrow_down: |
   | [...atus/compute/service/ComputeNodeStatusService.java](https://codecov.io/gh/apache/shardingsphere/pull/24441?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-bW9kZS90eXBlL2NsdXN0ZXIvY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvbW9kZS9tYW5hZ2VyL2NsdXN0ZXIvY29vcmRpbmF0b3IvcmVnaXN0cnkvc3RhdHVzL2NvbXB1dGUvc2VydmljZS9Db21wdXRlTm9kZVN0YXR1c1NlcnZpY2UuamF2YQ==) | `71.11% <ø> (ø)` | |
   | [...dingsphere/proxy/backend/context/ProxyContext.java](https://codecov.io/gh/apache/shardingsphere/pull/24441?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cHJveHkvYmFja2VuZC9jb3JlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9wcm94eS9iYWNrZW5kL2NvbnRleHQvUHJveHlDb250ZXh0LmphdmE=) | `90.00% <ø> (ø)` | |
   | [...ere/proxy/backend/exception/ReadOnlyException.java](https://codecov.io/gh/apache/shardingsphere/pull/24441?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cHJveHkvYmFja2VuZC9jb3JlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9wcm94eS9iYWNrZW5kL2V4Y2VwdGlvbi9SZWFkT25seUV4Y2VwdGlvbi5qYXZh) | `0.00% <0.00%> (ø)` | |
   | [.../proxy/backend/exception/UnavailableException.java](https://codecov.io/gh/apache/shardingsphere/pull/24441?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cHJveHkvYmFja2VuZC9jb3JlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9wcm94eS9iYWNrZW5kL2V4Y2VwdGlvbi9VbmF2YWlsYWJsZUV4Y2VwdGlvbi5qYXZh) | `0.00% <0.00%> (ø)` | |
   | ... and [24 more](https://codecov.io/gh/apache/shardingsphere/pull/24441?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   :mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   


-- 
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: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [shardingsphere] RaigorJiang commented on a diff in pull request #24441: Add DistSQL LOCK CLUSTER

Posted by "RaigorJiang (via GitHub)" <gi...@apache.org>.
RaigorJiang commented on code in PR #24441:
URL: https://github.com/apache/shardingsphere/pull/24441#discussion_r1125986452


##########
proxy/backend/core/src/main/java/org/apache/shardingsphere/proxy/backend/state/impl/UnavailableProxyState.java:
##########
@@ -0,0 +1,47 @@
+/*
+ * 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.shardingsphere.proxy.backend.state.impl;
+
+import org.apache.shardingsphere.distsql.parser.statement.ral.RALStatement;
+import org.apache.shardingsphere.proxy.backend.exception.UnavailableException;
+import org.apache.shardingsphere.proxy.backend.state.spi.ProxyClusterState;
+import org.apache.shardingsphere.sql.parser.sql.common.statement.SQLStatement;
+import org.apache.shardingsphere.sql.parser.sql.common.statement.ddl.DDLStatement;
+import org.apache.shardingsphere.sql.parser.sql.common.statement.dml.DMLStatement;
+
+/**
+ * Unavailable proxy state.
+ */
+public final class UnavailableProxyState implements ProxyClusterState {
+    
+    @Override
+    public void check(final SQLStatement sqlStatement) {
+        if (isUnsupportedStatement(sqlStatement)) {
+            throw new UnavailableException();
+        }
+    }
+    
+    private boolean isUnsupportedStatement(final SQLStatement sqlStatement) {
+        return sqlStatement instanceof DMLStatement || sqlStatement instanceof DDLStatement || sqlStatement instanceof RALStatement;

Review Comment:
   Why check RAL but not RDL here  🤔



-- 
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: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [shardingsphere] RaigorJiang commented on a diff in pull request #24441: Add DistSQL LOCK CLUSTER

Posted by "RaigorJiang (via GitHub)" <gi...@apache.org>.
RaigorJiang commented on code in PR #24441:
URL: https://github.com/apache/shardingsphere/pull/24441#discussion_r1125682619


##########
infra/common/src/main/java/org/apache/shardingsphere/infra/state/cluster/ClusterStateContext.java:
##########
@@ -0,0 +1,44 @@
+/*
+ * 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.shardingsphere.infra.state.cluster;
+
+import lombok.Getter;
+
+/**
+ * Cluster state context.
+ */
+public final class ClusterStateContext {
+    
+    @Getter
+    private ClusterStateType currentState = ClusterStateType.OK;
+    
+    /**
+     * Switch state.
+     * 
+     * @param status status
+     */
+    public void switchState(final ClusterStateType status) {
+        if (currentState == status) {
+            return;
+        }
+        if (currentState != ClusterStateType.OK && ClusterStateType.OK != status) {

Review Comment:
   Enum should be on the left



-- 
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: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [shardingsphere] Pace2Car commented on a diff in pull request #24441: Add DistSQL LOCK CLUSTER

Posted by "Pace2Car (via GitHub)" <gi...@apache.org>.
Pace2Car commented on code in PR #24441:
URL: https://github.com/apache/shardingsphere/pull/24441#discussion_r1125831220


##########
infra/common/src/main/java/org/apache/shardingsphere/infra/state/cluster/ClusterStateContext.java:
##########
@@ -0,0 +1,44 @@
+/*
+ * 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.shardingsphere.infra.state.cluster;
+
+import lombok.Getter;
+
+/**
+ * Cluster state context.
+ */
+public final class ClusterStateContext {
+    
+    @Getter
+    private ClusterStateType currentState = ClusterStateType.OK;
+    
+    /**
+     * Switch state.
+     * 
+     * @param status status
+     */
+    public void switchState(final ClusterStateType status) {
+        if (currentState == status) {
+            return;
+        }
+        if (currentState != ClusterStateType.OK && ClusterStateType.OK != status) {

Review Comment:
   I have fixed this, please review again.



##########
infra/common/src/main/java/org/apache/shardingsphere/infra/state/cluster/ClusterStateType.java:
##########
@@ -0,0 +1,26 @@
+/*
+ * 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.shardingsphere.infra.state.cluster;
+
+/**
+ * Cluster state type.
+ */
+public enum ClusterStateType {

Review Comment:
   I have fixed this, please review again.



-- 
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: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [shardingsphere] Pace2Car commented on a diff in pull request #24441: Add DistSQL LOCK CLUSTER

Posted by "Pace2Car (via GitHub)" <gi...@apache.org>.
Pace2Car commented on code in PR #24441:
URL: https://github.com/apache/shardingsphere/pull/24441#discussion_r1125831073


##########
infra/common/src/main/java/org/apache/shardingsphere/infra/state/cluster/ClusterStateContext.java:
##########
@@ -0,0 +1,44 @@
+/*
+ * 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.shardingsphere.infra.state.cluster;
+
+import lombok.Getter;
+
+/**
+ * Cluster state context.
+ */
+public final class ClusterStateContext {
+    
+    @Getter
+    private ClusterStateType currentState = ClusterStateType.OK;
+    
+    /**
+     * Switch state.
+     * 
+     * @param status status
+     */
+    public void switchState(final ClusterStateType status) {

Review Comment:
   I have fixed this, please review again.



-- 
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: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [shardingsphere] RaigorJiang merged pull request #24441: Add DistSQL LOCK CLUSTER

Posted by "RaigorJiang (via GitHub)" <gi...@apache.org>.
RaigorJiang merged PR #24441:
URL: https://github.com/apache/shardingsphere/pull/24441


-- 
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: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [shardingsphere] RaigorJiang commented on a diff in pull request #24441: Add DistSQL LOCK CLUSTER

Posted by "RaigorJiang (via GitHub)" <gi...@apache.org>.
RaigorJiang commented on code in PR #24441:
URL: https://github.com/apache/shardingsphere/pull/24441#discussion_r1125682519


##########
infra/common/src/main/java/org/apache/shardingsphere/infra/state/cluster/ClusterStateContext.java:
##########
@@ -0,0 +1,44 @@
+/*
+ * 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.shardingsphere.infra.state.cluster;
+
+import lombok.Getter;
+
+/**
+ * Cluster state context.
+ */
+public final class ClusterStateContext {
+    
+    @Getter
+    private ClusterStateType currentState = ClusterStateType.OK;
+    
+    /**
+     * Switch state.
+     * 
+     * @param status status
+     */
+    public void switchState(final ClusterStateType status) {

Review Comment:
   It seems that `state` and `status` express the same meaning, can they be named uniformly?



-- 
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: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [shardingsphere] RaigorJiang commented on a diff in pull request #24441: Add DistSQL LOCK CLUSTER

Posted by "RaigorJiang (via GitHub)" <gi...@apache.org>.
RaigorJiang commented on code in PR #24441:
URL: https://github.com/apache/shardingsphere/pull/24441#discussion_r1125683501


##########
proxy/backend/spi/pom.xml:
##########
@@ -0,0 +1,37 @@
+<?xml version="1.0" encoding="UTF-8"?>

Review Comment:
   Is it necessary to add this module `spi`



-- 
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: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [shardingsphere] RaigorJiang commented on a diff in pull request #24441: Add DistSQL LOCK CLUSTER

Posted by "RaigorJiang (via GitHub)" <gi...@apache.org>.
RaigorJiang commented on code in PR #24441:
URL: https://github.com/apache/shardingsphere/pull/24441#discussion_r1125682759


##########
infra/common/src/main/java/org/apache/shardingsphere/infra/state/cluster/ClusterStateType.java:
##########
@@ -0,0 +1,26 @@
+/*
+ * 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.shardingsphere.infra.state.cluster;
+
+/**
+ * Cluster state type.
+ */
+public enum ClusterStateType {

Review Comment:
   `ClusterState` may be more accurate than `ClusterStateType`



-- 
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: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [shardingsphere] Pace2Car commented on a diff in pull request #24441: Add DistSQL LOCK CLUSTER

Posted by "Pace2Car (via GitHub)" <gi...@apache.org>.
Pace2Car commented on code in PR #24441:
URL: https://github.com/apache/shardingsphere/pull/24441#discussion_r1126039097


##########
proxy/backend/core/src/main/java/org/apache/shardingsphere/proxy/backend/state/impl/ReadOnlyProxyState.java:
##########
@@ -0,0 +1,47 @@
+/*
+ * 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.shardingsphere.proxy.backend.state.impl;
+
+import org.apache.shardingsphere.proxy.backend.exception.ReadOnlyException;
+import org.apache.shardingsphere.proxy.backend.state.spi.ProxyClusterState;
+import org.apache.shardingsphere.sql.parser.sql.common.statement.SQLStatement;
+import org.apache.shardingsphere.sql.parser.sql.common.statement.dal.ShowStatement;
+import org.apache.shardingsphere.sql.parser.sql.common.statement.dml.SelectStatement;
+import org.apache.shardingsphere.sql.parser.sql.dialect.statement.mysql.dal.MySQLShowDatabasesStatement;
+
+/**
+ * ReadOnly proxy state.
+ */
+public final class ReadOnlyProxyState implements ProxyClusterState {
+    
+    @Override
+    public void check(final SQLStatement sqlStatement) {
+        if (isWriteStatement(sqlStatement)) {
+            throw new ReadOnlyException();
+        }
+    }
+    
+    private boolean isWriteStatement(final SQLStatement sqlStatement) {
+        return !(sqlStatement instanceof SelectStatement) && !(sqlStatement instanceof ShowStatement) && !(sqlStatement instanceof MySQLShowDatabasesStatement);

Review Comment:
   Thanks for point out, I have fixed.



##########
proxy/backend/core/src/main/java/org/apache/shardingsphere/proxy/backend/state/impl/UnavailableProxyState.java:
##########
@@ -0,0 +1,47 @@
+/*
+ * 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.shardingsphere.proxy.backend.state.impl;
+
+import org.apache.shardingsphere.distsql.parser.statement.ral.RALStatement;
+import org.apache.shardingsphere.proxy.backend.exception.UnavailableException;
+import org.apache.shardingsphere.proxy.backend.state.spi.ProxyClusterState;
+import org.apache.shardingsphere.sql.parser.sql.common.statement.SQLStatement;
+import org.apache.shardingsphere.sql.parser.sql.common.statement.ddl.DDLStatement;
+import org.apache.shardingsphere.sql.parser.sql.common.statement.dml.DMLStatement;
+
+/**
+ * Unavailable proxy state.
+ */
+public final class UnavailableProxyState implements ProxyClusterState {
+    
+    @Override
+    public void check(final SQLStatement sqlStatement) {
+        if (isUnsupportedStatement(sqlStatement)) {
+            throw new UnavailableException();
+        }
+    }
+    
+    private boolean isUnsupportedStatement(final SQLStatement sqlStatement) {
+        return sqlStatement instanceof DMLStatement || sqlStatement instanceof DDLStatement || sqlStatement instanceof RALStatement;

Review Comment:
   Thanks for point out, I have fixed.



-- 
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: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [shardingsphere] Pace2Car commented on a diff in pull request #24441: Add DistSQL LOCK CLUSTER

Posted by "Pace2Car (via GitHub)" <gi...@apache.org>.
Pace2Car commented on code in PR #24441:
URL: https://github.com/apache/shardingsphere/pull/24441#discussion_r1125832486


##########
proxy/backend/spi/pom.xml:
##########
@@ -0,0 +1,37 @@
+<?xml version="1.0" encoding="UTF-8"?>

Review Comment:
   Yes, it is used to support various types of locking and checking operations.



-- 
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: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org