You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@shardingsphere.apache.org by zh...@apache.org on 2020/12/09 05:04:47 UTC
[shardingsphere] branch master updated: Add lock wait timeout
configuration (#8535)
This is an automated email from the ASF dual-hosted git repository.
zhangliang pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/shardingsphere.git
The following commit(s) were added to refs/heads/master by this push:
new 9af23fa Add lock wait timeout configuration (#8535)
9af23fa is described below
commit 9af23fa447a8a9670392e07c97b707f095c5ac16
Author: Haoran Meng <me...@gmail.com>
AuthorDate: Wed Dec 9 13:04:21 2020 +0800
Add lock wait timeout configuration (#8535)
* Add lock wait timeout configuration
---
.../governance/core/lock/GovernanceLockStrategy.java | 4 ++--
.../shardingsphere/governance/core/lock/LockCenter.java | 6 +++---
.../governance/core/lock/GovernanceLockStrategyTest.java | 5 +++--
.../shardingsphere/governance/core/lock/LockCenterTest.java | 4 ++--
.../infra/config/properties/ConfigurationPropertyKey.java | 7 ++++++-
.../org/apache/shardingsphere/infra/lock/LockContext.java | 7 +++----
.../org/apache/shardingsphere/infra/lock/LockStrategy.java | 3 ++-
.../shardingsphere/infra/lock/StandardLockStrategy.java | 11 +++++++++--
.../org/apache/shardingsphere/infra/lock/LockContextTest.java | 4 +---
.../shardingsphere/infra/lock/StandardLockStrategyTest.java | 4 ++--
.../backend/communication/DatabaseCommunicationEngine.java | 11 ++---------
.../src/main/resources/conf/server.yaml | 1 +
.../proxy/frontend/state/impl/LockProxyState.java | 2 +-
13 files changed, 37 insertions(+), 32 deletions(-)
diff --git a/shardingsphere-governance/shardingsphere-governance-core/src/main/java/org/apache/shardingsphere/governance/core/lock/GovernanceLockStrategy.java b/shardingsphere-governance/shardingsphere-governance-core/src/main/java/org/apache/shardingsphere/governance/core/lock/GovernanceLockStrategy.java
index 62acd86..ba5b3ad 100644
--- a/shardingsphere-governance/shardingsphere-governance-core/src/main/java/org/apache/shardingsphere/governance/core/lock/GovernanceLockStrategy.java
+++ b/shardingsphere-governance/shardingsphere-governance-core/src/main/java/org/apache/shardingsphere/governance/core/lock/GovernanceLockStrategy.java
@@ -39,8 +39,8 @@ public final class GovernanceLockStrategy implements LockStrategy {
}
@Override
- public boolean tryLock() {
- return governanceFacade.getLockCenter().tryGlobalLock();
+ public boolean tryLock(final Long timeout) {
+ return governanceFacade.getLockCenter().tryGlobalLock(timeout);
}
@Override
diff --git a/shardingsphere-governance/shardingsphere-governance-core/src/main/java/org/apache/shardingsphere/governance/core/lock/LockCenter.java b/shardingsphere-governance/shardingsphere-governance-core/src/main/java/org/apache/shardingsphere/governance/core/lock/LockCenter.java
index 863883e..8b96d1c 100644
--- a/shardingsphere-governance/shardingsphere-governance-core/src/main/java/org/apache/shardingsphere/governance/core/lock/LockCenter.java
+++ b/shardingsphere-governance/shardingsphere-governance-core/src/main/java/org/apache/shardingsphere/governance/core/lock/LockCenter.java
@@ -75,11 +75,11 @@ public final class LockCenter {
/**
* Try to get global lock.
*
+ * @param timeout the maximum time in milliseconds to acquire lock
* @return true if get the lock, false if not
*/
- public boolean tryGlobalLock() {
- // TODO timeout and retry
- return registryRepository.tryLock(5, TimeUnit.SECONDS);
+ public boolean tryGlobalLock(final Long timeout) {
+ return registryRepository.tryLock(timeout, TimeUnit.MILLISECONDS);
}
/**
diff --git a/shardingsphere-governance/shardingsphere-governance-core/src/test/java/org/apache/shardingsphere/governance/core/lock/GovernanceLockStrategyTest.java b/shardingsphere-governance/shardingsphere-governance-core/src/test/java/org/apache/shardingsphere/governance/core/lock/GovernanceLockStrategyTest.java
index e8cfed3..5f7b321 100644
--- a/shardingsphere-governance/shardingsphere-governance-core/src/test/java/org/apache/shardingsphere/governance/core/lock/GovernanceLockStrategyTest.java
+++ b/shardingsphere-governance/shardingsphere-governance-core/src/test/java/org/apache/shardingsphere/governance/core/lock/GovernanceLockStrategyTest.java
@@ -25,6 +25,7 @@ import org.junit.runner.RunWith;
import org.mockito.Mock;
import org.mockito.junit.MockitoJUnitRunner;
+import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
@@ -47,8 +48,8 @@ public final class GovernanceLockStrategyTest {
@Test
public void assertTryLock() {
- lockStrategy.tryLock();
- verify(lockCenter).tryGlobalLock();
+ lockStrategy.tryLock(50L);
+ verify(lockCenter).tryGlobalLock(eq(50L));
}
@Test
diff --git a/shardingsphere-governance/shardingsphere-governance-core/src/test/java/org/apache/shardingsphere/governance/core/lock/LockCenterTest.java b/shardingsphere-governance/shardingsphere-governance-core/src/test/java/org/apache/shardingsphere/governance/core/lock/LockCenterTest.java
index d3841b9..1ea993d 100644
--- a/shardingsphere-governance/shardingsphere-governance-core/src/test/java/org/apache/shardingsphere/governance/core/lock/LockCenterTest.java
+++ b/shardingsphere-governance/shardingsphere-governance-core/src/test/java/org/apache/shardingsphere/governance/core/lock/LockCenterTest.java
@@ -65,8 +65,8 @@ public final class LockCenterTest {
@Test
public void assertTryGlobalLock() {
- lockCenter.tryGlobalLock();
- verify(registryRepository).tryLock(eq(5L), eq(TimeUnit.SECONDS));
+ lockCenter.tryGlobalLock(50L);
+ verify(registryRepository).tryLock(eq(50L), eq(TimeUnit.MILLISECONDS));
}
@Test
diff --git a/shardingsphere-infra/shardingsphere-infra-common/src/main/java/org/apache/shardingsphere/infra/config/properties/ConfigurationPropertyKey.java b/shardingsphere-infra/shardingsphere-infra-common/src/main/java/org/apache/shardingsphere/infra/config/properties/ConfigurationPropertyKey.java
index 469854b..a09db77 100644
--- a/shardingsphere-infra/shardingsphere-infra-common/src/main/java/org/apache/shardingsphere/infra/config/properties/ConfigurationPropertyKey.java
+++ b/shardingsphere-infra/shardingsphere-infra-common/src/main/java/org/apache/shardingsphere/infra/config/properties/ConfigurationPropertyKey.java
@@ -96,7 +96,12 @@ public enum ConfigurationPropertyKey implements TypedPropertyKey {
/**
* Whether enable hint for ShardingSphere-Proxy.
*/
- PROXY_HINT_ENABLED("proxy-hint-enabled", String.valueOf(Boolean.FALSE), boolean.class);
+ PROXY_HINT_ENABLED("proxy-hint-enabled", String.valueOf(Boolean.FALSE), boolean.class),
+
+ /**
+ * The length of time in milliseconds an SQL waits for a global lock before giving up.
+ */
+ LOCK_WAIT_TIMEOUT_MILLISECONDS("lock-wait-timeout-milliseconds", String.valueOf(5000L), long.class);
private final String key;
diff --git a/shardingsphere-infra/shardingsphere-infra-common/src/main/java/org/apache/shardingsphere/infra/lock/LockContext.java b/shardingsphere-infra/shardingsphere-infra-common/src/main/java/org/apache/shardingsphere/infra/lock/LockContext.java
index 65987a3..37eb48d 100644
--- a/shardingsphere-infra/shardingsphere-infra-common/src/main/java/org/apache/shardingsphere/infra/lock/LockContext.java
+++ b/shardingsphere-infra/shardingsphere-infra-common/src/main/java/org/apache/shardingsphere/infra/lock/LockContext.java
@@ -38,8 +38,6 @@ public final class LockContext {
private static final Condition CONDITION = LOCK.newCondition();
- private static final long TIME_OUT_SECONDS = 50L;
-
/**
* Init lock strategy.
*
@@ -61,12 +59,13 @@ public final class LockContext {
/**
* Waiting for unlock.
*
+ * @param timeout the maximum time in milliseconds to wait
* @return false if wait timeout exceeded, else true
*/
- public static boolean await() {
+ public static boolean await(final Long timeout) {
LOCK.lock();
try {
- return CONDITION.await(TIME_OUT_SECONDS, TimeUnit.SECONDS);
+ return CONDITION.await(timeout, TimeUnit.MILLISECONDS);
// CHECKSTYLE:OFF
} catch (InterruptedException e) {
// CHECKSTYLE:ON
diff --git a/shardingsphere-infra/shardingsphere-infra-common/src/main/java/org/apache/shardingsphere/infra/lock/LockStrategy.java b/shardingsphere-infra/shardingsphere-infra-common/src/main/java/org/apache/shardingsphere/infra/lock/LockStrategy.java
index 2a5c389..0c9cec2 100644
--- a/shardingsphere-infra/shardingsphere-infra-common/src/main/java/org/apache/shardingsphere/infra/lock/LockStrategy.java
+++ b/shardingsphere-infra/shardingsphere-infra-common/src/main/java/org/apache/shardingsphere/infra/lock/LockStrategy.java
@@ -25,9 +25,10 @@ public interface LockStrategy {
/**
* Try to get lock.
*
+ * @param timeout the maximum time in milliseconds to acquire lock
* @return true if get the lock, false if not
*/
- boolean tryLock();
+ boolean tryLock(Long timeout);
/**
* Release lock.
diff --git a/shardingsphere-infra/shardingsphere-infra-common/src/main/java/org/apache/shardingsphere/infra/lock/StandardLockStrategy.java b/shardingsphere-infra/shardingsphere-infra-common/src/main/java/org/apache/shardingsphere/infra/lock/StandardLockStrategy.java
index a8d18ab..9395aba 100644
--- a/shardingsphere-infra/shardingsphere-infra-common/src/main/java/org/apache/shardingsphere/infra/lock/StandardLockStrategy.java
+++ b/shardingsphere-infra/shardingsphere-infra-common/src/main/java/org/apache/shardingsphere/infra/lock/StandardLockStrategy.java
@@ -21,6 +21,7 @@ import org.apache.shardingsphere.infra.state.StateContext;
import org.apache.shardingsphere.infra.state.StateEvent;
import org.apache.shardingsphere.infra.state.StateType;
+import java.util.concurrent.TimeUnit;
import java.util.concurrent.locks.ReentrantLock;
/**
@@ -31,8 +32,14 @@ public final class StandardLockStrategy implements LockStrategy {
private final ReentrantLock lock = new ReentrantLock();
@Override
- public boolean tryLock() {
- boolean result = lock.tryLock();
+ public boolean tryLock(final Long timeout) {
+ boolean result = false;
+ try {
+ result = lock.tryLock(timeout, TimeUnit.MILLISECONDS);
+ // CHECKSTYLE:OFF
+ } catch (final InterruptedException e) {
+ // CHECKSTYLE:ON
+ }
if (result) {
StateContext.switchState(new StateEvent(StateType.LOCK, true));
}
diff --git a/shardingsphere-infra/shardingsphere-infra-common/src/test/java/org/apache/shardingsphere/infra/lock/LockContextTest.java b/shardingsphere-infra/shardingsphere-infra-common/src/test/java/org/apache/shardingsphere/infra/lock/LockContextTest.java
index 2cb4ac2..255b586 100644
--- a/shardingsphere-infra/shardingsphere-infra-common/src/test/java/org/apache/shardingsphere/infra/lock/LockContextTest.java
+++ b/shardingsphere-infra/shardingsphere-infra-common/src/test/java/org/apache/shardingsphere/infra/lock/LockContextTest.java
@@ -44,7 +44,6 @@ public final class LockContextTest {
@Test
public void assetAwait() {
- long startTime = System.currentTimeMillis();
new Thread(() -> {
try {
TimeUnit.MILLISECONDS.sleep(200L);
@@ -54,8 +53,7 @@ public final class LockContextTest {
}
LockContext.signalAll();
}).start();
- boolean result = LockContext.await();
+ boolean result = LockContext.await(400L);
assertTrue(result);
- assertTrue(System.currentTimeMillis() - startTime >= 200L);
}
}
diff --git a/shardingsphere-infra/shardingsphere-infra-common/src/test/java/org/apache/shardingsphere/infra/lock/StandardLockStrategyTest.java b/shardingsphere-infra/shardingsphere-infra-common/src/test/java/org/apache/shardingsphere/infra/lock/StandardLockStrategyTest.java
index 3f7fc91..90fa920 100644
--- a/shardingsphere-infra/shardingsphere-infra-common/src/test/java/org/apache/shardingsphere/infra/lock/StandardLockStrategyTest.java
+++ b/shardingsphere-infra/shardingsphere-infra-common/src/test/java/org/apache/shardingsphere/infra/lock/StandardLockStrategyTest.java
@@ -31,14 +31,14 @@ public final class StandardLockStrategyTest {
@Test
public void assertTryLock() {
- assertTrue(lockStrategy.tryLock());
+ assertTrue(lockStrategy.tryLock(50L));
assertThat(StateContext.getCurrentState(), is(StateType.LOCK));
lockStrategy.releaseLock();
}
@Test
public void assertReleaseLock() {
- assertTrue(lockStrategy.tryLock());
+ assertTrue(lockStrategy.tryLock(50L));
assertThat(StateContext.getCurrentState(), is(StateType.LOCK));
lockStrategy.releaseLock();
assertThat(StateContext.getCurrentState(), is(StateType.OK));
diff --git a/shardingsphere-proxy/shardingsphere-proxy-backend/src/main/java/org/apache/shardingsphere/proxy/backend/communication/DatabaseCommunicationEngine.java b/shardingsphere-proxy/shardingsphere-proxy-backend/src/main/java/org/apache/shardingsphere/proxy/backend/communication/DatabaseCommunicationEngine.java
index eb33e07..bca6496 100644
--- a/shardingsphere-proxy/shardingsphere-proxy-backend/src/main/java/org/apache/shardingsphere/proxy/backend/communication/DatabaseCommunicationEngine.java
+++ b/shardingsphere-proxy/shardingsphere-proxy-backend/src/main/java/org/apache/shardingsphere/proxy/backend/communication/DatabaseCommunicationEngine.java
@@ -122,7 +122,7 @@ public final class DatabaseCommunicationEngine {
private void lockForDDL(final ExecutionContext executionContext) {
if (needLock(executionContext)) {
- if (!LockContext.getLockStrategy().tryLock()) {
+ if (!LockContext.getLockStrategy().tryLock(ProxyContext.getInstance().getMetaDataContexts().getProps().<Long>getValue(ConfigurationPropertyKey.LOCK_WAIT_TIMEOUT_MILLISECONDS))) {
throw new LockWaitTimeoutException();
}
checkLock();
@@ -130,11 +130,7 @@ public final class DatabaseCommunicationEngine {
}
private boolean needLock(final ExecutionContext executionContext) {
- SQLStatement sqlStatement = executionContext.getSqlStatementContext().getSqlStatement();
- if (null == sqlStatement) {
- return false;
- }
- return SchemaRefresherFactory.newInstance(sqlStatement).isPresent();
+ return SchemaRefresherFactory.newInstance(executionContext.getSqlStatementContext().getSqlStatement()).isPresent();
}
private void checkLock() {
@@ -185,9 +181,6 @@ public final class DatabaseCommunicationEngine {
@SuppressWarnings({"unchecked", "rawtypes"})
private void refreshSchema(final ExecutionContext executionContext) throws SQLException {
SQLStatement sqlStatement = executionContext.getSqlStatementContext().getSqlStatement();
- if (null == sqlStatement) {
- return;
- }
Optional<SchemaRefresher> schemaRefresher = SchemaRefresherFactory.newInstance(sqlStatement);
if (schemaRefresher.isPresent()) {
try {
diff --git a/shardingsphere-proxy/shardingsphere-proxy-bootstrap/src/main/resources/conf/server.yaml b/shardingsphere-proxy/shardingsphere-proxy-bootstrap/src/main/resources/conf/server.yaml
index a72437e..7e52f84 100644
--- a/shardingsphere-proxy/shardingsphere-proxy-bootstrap/src/main/resources/conf/server.yaml
+++ b/shardingsphere-proxy/shardingsphere-proxy-bootstrap/src/main/resources/conf/server.yaml
@@ -55,3 +55,4 @@
# query-with-cipher-column: true
# sql-show: false
# check-table-metadata-enabled: false
+# lock-wait-timeout-milliseconds: 5000 # The maximum time to wait for a lock
diff --git a/shardingsphere-proxy/shardingsphere-proxy-frontend/shardingsphere-proxy-frontend-core/src/main/java/org/apache/shardingsphere/proxy/frontend/state/impl/LockProxyState.java b/shardingsphere-proxy/shardingsphere-proxy-frontend/shardingsphere-proxy-frontend-core/src/main/java/org/apache/shardingsphere/proxy/frontend/state/impl/LockProxyState.java
index 989df9d..07c5af4 100644
--- a/shardingsphere-proxy/shardingsphere-proxy-frontend/shardingsphere-proxy-frontend-core/src/main/java/org/apache/shardingsphere/proxy/frontend/state/impl/LockProxyState.java
+++ b/shardingsphere-proxy/shardingsphere-proxy-frontend/shardingsphere-proxy-frontend-core/src/main/java/org/apache/shardingsphere/proxy/frontend/state/impl/LockProxyState.java
@@ -54,7 +54,7 @@ public final class LockProxyState implements ProxyState {
}
private void block(final ChannelHandlerContext context, final DatabaseProtocolFrontendEngine databaseProtocolFrontendEngine) {
- if (!LockContext.await()) {
+ if (!LockContext.await(ProxyContext.getInstance().getMetaDataContexts().getProps().<Long>getValue(ConfigurationPropertyKey.LOCK_WAIT_TIMEOUT_MILLISECONDS))) {
doError(context, databaseProtocolFrontendEngine, new LockWaitTimeoutException());
return;
}