You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@shardingsphere.apache.org by pa...@apache.org on 2021/02/13 04:24:01 UTC
[shardingsphere] branch master updated: Fixes deadlock when DDL
execute failed (#9414)
This is an automated email from the ASF dual-hosted git repository.
panjuan 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 9a24c48 Fixes deadlock when DDL execute failed (#9414)
9a24c48 is described below
commit 9a24c4847c7bb0ee49d2681cf87bd8600400b298
Author: Haoran Meng <me...@gmail.com>
AuthorDate: Sat Feb 13 12:23:40 2021 +0800
Fixes deadlock when DDL execute failed (#9414)
---
.../context/metadata/GovernanceMetaDataContexts.java | 12 ++++++------
.../{UnlockEvent.java => GlobalLockReleasedEvent.java} | 4 ++--
.../governance/core/lock/GovernanceLockContext.java | 8 ++++----
.../governance/core/registry/RegistryCenter.java | 7 +++++--
.../registry/listener/GlobalLockChangedListener.java | 12 +++++++++++-
.../core/registry/listener/RegistryListenerManager.java | 2 +-
.../governance/core/registry/RegistryCenterTest.java | 1 -
.../listener/GlobalLockChangedListenerTest.java | 17 ++++++++++++++++-
.../driver/executor/DriverJDBCExecutor.java | 5 +++--
.../communication/DatabaseCommunicationEngine.java | 7 ++++---
10 files changed, 52 insertions(+), 23 deletions(-)
diff --git a/shardingsphere-governance/shardingsphere-governance-context/src/main/java/org/apache/shardingsphere/governance/context/metadata/GovernanceMetaDataContexts.java b/shardingsphere-governance/shardingsphere-governance-context/src/main/java/org/apache/shardingsphere/governance/context/metadata/GovernanceMetaDataContexts.java
index 24acdb8..f76d423 100644
--- a/shardingsphere-governance/shardingsphere-governance-context/src/main/java/org/apache/shardingsphere/governance/context/metadata/GovernanceMetaDataContexts.java
+++ b/shardingsphere-governance/shardingsphere-governance-context/src/main/java/org/apache/shardingsphere/governance/context/metadata/GovernanceMetaDataContexts.java
@@ -22,7 +22,7 @@ import com.google.common.eventbus.Subscribe;
import org.apache.shardingsphere.governance.core.event.model.auth.UserRuleChangedEvent;
import org.apache.shardingsphere.governance.core.event.model.datasource.DataSourceChangeCompletedEvent;
import org.apache.shardingsphere.governance.core.event.model.datasource.DataSourceChangedEvent;
-import org.apache.shardingsphere.governance.core.event.model.lock.UnlockEvent;
+import org.apache.shardingsphere.governance.core.event.model.lock.GlobalLockReleasedEvent;
import org.apache.shardingsphere.governance.core.event.model.metadata.MetaDataChangedEvent;
import org.apache.shardingsphere.governance.core.event.model.metadata.MetaDataDeletedEvent;
import org.apache.shardingsphere.governance.core.event.model.metadata.MetaDataPersistedEvent;
@@ -34,10 +34,6 @@ import org.apache.shardingsphere.governance.core.lock.GovernanceLockContext;
import org.apache.shardingsphere.governance.core.registry.event.DisabledStateChangedEvent;
import org.apache.shardingsphere.governance.core.registry.event.PrimaryStateChangedEvent;
import org.apache.shardingsphere.governance.core.registry.schema.GovernanceSchema;
-import org.apache.shardingsphere.infra.metadata.auth.Authentication;
-import org.apache.shardingsphere.infra.metadata.auth.model.user.ShardingSphereUser;
-import org.apache.shardingsphere.infra.metadata.auth.builtin.DefaultAuthentication;
-import org.apache.shardingsphere.infra.metadata.auth.model.privilege.ShardingSpherePrivilege;
import org.apache.shardingsphere.infra.config.RuleConfiguration;
import org.apache.shardingsphere.infra.config.datasource.DataSourceConfiguration;
import org.apache.shardingsphere.infra.config.datasource.DataSourceConverter;
@@ -49,6 +45,10 @@ import org.apache.shardingsphere.infra.eventbus.ShardingSphereEventBus;
import org.apache.shardingsphere.infra.executor.kernel.ExecutorEngine;
import org.apache.shardingsphere.infra.lock.LockContext;
import org.apache.shardingsphere.infra.metadata.ShardingSphereMetaData;
+import org.apache.shardingsphere.infra.metadata.auth.Authentication;
+import org.apache.shardingsphere.infra.metadata.auth.builtin.DefaultAuthentication;
+import org.apache.shardingsphere.infra.metadata.auth.model.privilege.ShardingSpherePrivilege;
+import org.apache.shardingsphere.infra.metadata.auth.model.user.ShardingSphereUser;
import org.apache.shardingsphere.infra.metadata.schema.ShardingSphereSchema;
import org.apache.shardingsphere.infra.optimize.context.CalciteContextFactory;
import org.apache.shardingsphere.infra.rule.ShardingSphereRule;
@@ -229,7 +229,7 @@ public final class GovernanceMetaDataContexts implements MetaDataContexts {
}
metaDataContexts = new StandardMetaDataContexts(newMetaDataMap, metaDataContexts.getExecutorEngine(), metaDataContexts.getAuthentication(), metaDataContexts.getProps());
} finally {
- ShardingSphereEventBus.getInstance().post(new UnlockEvent());
+ ShardingSphereEventBus.getInstance().post(new GlobalLockReleasedEvent());
}
}
diff --git a/shardingsphere-governance/shardingsphere-governance-core/src/main/java/org/apache/shardingsphere/governance/core/event/model/lock/UnlockEvent.java b/shardingsphere-governance/shardingsphere-governance-core/src/main/java/org/apache/shardingsphere/governance/core/event/model/lock/GlobalLockReleasedEvent.java
similarity index 90%
rename from shardingsphere-governance/shardingsphere-governance-core/src/main/java/org/apache/shardingsphere/governance/core/event/model/lock/UnlockEvent.java
rename to shardingsphere-governance/shardingsphere-governance-core/src/main/java/org/apache/shardingsphere/governance/core/event/model/lock/GlobalLockReleasedEvent.java
index 3253bf9..f628af13 100644
--- a/shardingsphere-governance/shardingsphere-governance-core/src/main/java/org/apache/shardingsphere/governance/core/event/model/lock/UnlockEvent.java
+++ b/shardingsphere-governance/shardingsphere-governance-core/src/main/java/org/apache/shardingsphere/governance/core/event/model/lock/GlobalLockReleasedEvent.java
@@ -20,7 +20,7 @@ package org.apache.shardingsphere.governance.core.event.model.lock;
import org.apache.shardingsphere.governance.core.event.model.GovernanceEvent;
/**
- * Unlock event.
+ * Global lock released event.
*/
-public final class UnlockEvent implements GovernanceEvent {
+public final class GlobalLockReleasedEvent implements GovernanceEvent {
}
diff --git a/shardingsphere-governance/shardingsphere-governance-core/src/main/java/org/apache/shardingsphere/governance/core/lock/GovernanceLockContext.java b/shardingsphere-governance/shardingsphere-governance-core/src/main/java/org/apache/shardingsphere/governance/core/lock/GovernanceLockContext.java
index 72824e7..7b7e54c 100644
--- a/shardingsphere-governance/shardingsphere-governance-core/src/main/java/org/apache/shardingsphere/governance/core/lock/GovernanceLockContext.java
+++ b/shardingsphere-governance/shardingsphere-governance-core/src/main/java/org/apache/shardingsphere/governance/core/lock/GovernanceLockContext.java
@@ -19,7 +19,7 @@ package org.apache.shardingsphere.governance.core.lock;
import com.google.common.eventbus.Subscribe;
import org.apache.shardingsphere.governance.core.event.model.lock.GlobalLockAddedEvent;
-import org.apache.shardingsphere.governance.core.event.model.lock.UnlockEvent;
+import org.apache.shardingsphere.governance.core.event.model.lock.GlobalLockReleasedEvent;
import org.apache.shardingsphere.governance.core.registry.RegistryCenter;
import org.apache.shardingsphere.governance.core.registry.RegistryCenterNodeStatus;
import org.apache.shardingsphere.infra.eventbus.ShardingSphereEventBus;
@@ -64,11 +64,11 @@ public final class GovernanceLockContext extends AbstractLockContext {
/**
* Unlock instance.
- *
- * @param event unlock event
+ *
+ * @param event global lock released event
*/
@Subscribe
- public void unlock(final UnlockEvent event) {
+ public void unlock(final GlobalLockReleasedEvent event) {
ShardingSphereEventBus.getInstance().post(new StateEvent(StateType.LOCK, false));
registryCenter.persistInstanceData("");
signalAll();
diff --git a/shardingsphere-governance/shardingsphere-governance-core/src/main/java/org/apache/shardingsphere/governance/core/registry/RegistryCenter.java b/shardingsphere-governance/shardingsphere-governance-core/src/main/java/org/apache/shardingsphere/governance/core/registry/RegistryCenter.java
index 173d24f..591fe78 100644
--- a/shardingsphere-governance/shardingsphere-governance-core/src/main/java/org/apache/shardingsphere/governance/core/registry/RegistryCenter.java
+++ b/shardingsphere-governance/shardingsphere-governance-core/src/main/java/org/apache/shardingsphere/governance/core/registry/RegistryCenter.java
@@ -169,11 +169,14 @@ public final class RegistryCenter {
*/
public void releaseGlobalLock() {
repository.releaseLock(lockNode.getGlobalLockNodePath());
- repository.delete(lockNode.getGlobalLockNodePath());
}
private boolean checkLock() {
- return checkOrRetry(loadAllInstances());
+ boolean result = checkOrRetry(loadAllInstances());
+ if (!result) {
+ releaseGlobalLock();
+ }
+ return result;
}
private boolean checkOrRetry(final Collection<String> instanceIds) {
diff --git a/shardingsphere-governance/shardingsphere-governance-core/src/main/java/org/apache/shardingsphere/governance/core/registry/listener/GlobalLockChangedListener.java b/shardingsphere-governance/shardingsphere-governance-core/src/main/java/org/apache/shardingsphere/governance/core/registry/listener/GlobalLockChangedListener.java
index 4fad8eb..b14640c 100644
--- a/shardingsphere-governance/shardingsphere-governance-core/src/main/java/org/apache/shardingsphere/governance/core/registry/listener/GlobalLockChangedListener.java
+++ b/shardingsphere-governance/shardingsphere-governance-core/src/main/java/org/apache/shardingsphere/governance/core/registry/listener/GlobalLockChangedListener.java
@@ -17,12 +17,15 @@
package org.apache.shardingsphere.governance.core.registry.listener;
+import com.google.common.base.Joiner;
import org.apache.shardingsphere.governance.core.event.listener.PostGovernanceRepositoryEventListener;
import org.apache.shardingsphere.governance.core.event.model.GovernanceEvent;
import org.apache.shardingsphere.governance.core.event.model.lock.GlobalLockAddedEvent;
+import org.apache.shardingsphere.governance.core.event.model.lock.GlobalLockReleasedEvent;
import org.apache.shardingsphere.governance.core.lock.node.LockNode;
import org.apache.shardingsphere.governance.repository.api.RegistryRepository;
import org.apache.shardingsphere.governance.repository.api.listener.DataChangedEvent;
+import org.apache.shardingsphere.governance.repository.api.listener.DataChangedEvent.Type;
import java.util.Collections;
import java.util.Optional;
@@ -41,6 +44,13 @@ public final class GlobalLockChangedListener extends PostGovernanceRepositoryEve
@Override
protected Optional<GovernanceEvent> createEvent(final DataChangedEvent event) {
- return event.getKey().equals(lockNode.getGlobalLockNodePath()) ? Optional.of(new GlobalLockAddedEvent()) : Optional.empty();
+ if (event.getKey().startsWith(Joiner.on("/").join(lockNode.getGlobalLockNodePath(), ""))) {
+ if (event.getType() == Type.ADDED) {
+ return Optional.of(new GlobalLockAddedEvent());
+ } else if (event.getType() == Type.DELETED) {
+ return Optional.of(new GlobalLockReleasedEvent());
+ }
+ }
+ return Optional.empty();
}
}
diff --git a/shardingsphere-governance/shardingsphere-governance-core/src/main/java/org/apache/shardingsphere/governance/core/registry/listener/RegistryListenerManager.java b/shardingsphere-governance/shardingsphere-governance-core/src/main/java/org/apache/shardingsphere/governance/core/registry/listener/RegistryListenerManager.java
index 4172580..32ade59 100644
--- a/shardingsphere-governance/shardingsphere-governance-core/src/main/java/org/apache/shardingsphere/governance/core/registry/listener/RegistryListenerManager.java
+++ b/shardingsphere-governance/shardingsphere-governance-core/src/main/java/org/apache/shardingsphere/governance/core/registry/listener/RegistryListenerManager.java
@@ -45,6 +45,6 @@ public final class RegistryListenerManager {
public void initListeners() {
terminalStateChangedListener.watch(Type.UPDATED);
dataSourceStateChangedListener.watch(Type.UPDATED, Type.DELETED, Type.ADDED);
- globalLockChangedListener.watch(Type.ADDED);
+ globalLockChangedListener.watch(Type.ADDED, Type.DELETED);
}
}
diff --git a/shardingsphere-governance/shardingsphere-governance-core/src/test/java/org/apache/shardingsphere/governance/core/registry/RegistryCenterTest.java b/shardingsphere-governance/shardingsphere-governance-core/src/test/java/org/apache/shardingsphere/governance/core/registry/RegistryCenterTest.java
index 9bb1714..d23575b 100644
--- a/shardingsphere-governance/shardingsphere-governance-core/src/test/java/org/apache/shardingsphere/governance/core/registry/RegistryCenterTest.java
+++ b/shardingsphere-governance/shardingsphere-governance-core/src/test/java/org/apache/shardingsphere/governance/core/registry/RegistryCenterTest.java
@@ -94,6 +94,5 @@ public final class RegistryCenterTest {
public void assertReleaseGlobalLock() {
registryCenter.releaseGlobalLock();
verify(registryRepository).releaseLock(eq(new LockNode().getGlobalLockNodePath()));
- verify(registryRepository).delete(eq(new LockNode().getGlobalLockNodePath()));
}
}
diff --git a/shardingsphere-governance/shardingsphere-governance-core/src/test/java/org/apache/shardingsphere/governance/core/registry/listener/GlobalLockChangedListenerTest.java b/shardingsphere-governance/shardingsphere-governance-core/src/test/java/org/apache/shardingsphere/governance/core/registry/listener/GlobalLockChangedListenerTest.java
index 2ebd98d..daf9d01 100644
--- a/shardingsphere-governance/shardingsphere-governance-core/src/test/java/org/apache/shardingsphere/governance/core/registry/listener/GlobalLockChangedListenerTest.java
+++ b/shardingsphere-governance/shardingsphere-governance-core/src/test/java/org/apache/shardingsphere/governance/core/registry/listener/GlobalLockChangedListenerTest.java
@@ -19,6 +19,7 @@ package org.apache.shardingsphere.governance.core.registry.listener;
import org.apache.shardingsphere.governance.core.event.model.GovernanceEvent;
import org.apache.shardingsphere.governance.core.event.model.lock.GlobalLockAddedEvent;
+import org.apache.shardingsphere.governance.core.event.model.lock.GlobalLockReleasedEvent;
import org.apache.shardingsphere.governance.repository.api.RegistryRepository;
import org.apache.shardingsphere.governance.repository.api.listener.DataChangedEvent;
import org.apache.shardingsphere.governance.repository.api.listener.DataChangedEvent.Type;
@@ -30,6 +31,7 @@ import org.mockito.junit.MockitoJUnitRunner;
import java.util.Optional;
+import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
@RunWith(MockitoJUnitRunner.class)
@@ -46,9 +48,22 @@ public final class GlobalLockChangedListenerTest {
}
@Test
- public void createEvent() {
+ public void assertCreateEventWithInvalidPath() {
Optional<GovernanceEvent> actual = globalLockChangedListener.createEvent(new DataChangedEvent("/lock/glock", "", Type.ADDED));
+ assertFalse(actual.isPresent());
+ }
+
+ @Test
+ public void assertCreateAddedEvent() {
+ Optional<GovernanceEvent> actual = globalLockChangedListener.createEvent(new DataChangedEvent("/lock/glock/lock-test-id", "", Type.ADDED));
assertTrue(actual.isPresent());
assertTrue(actual.get() instanceof GlobalLockAddedEvent);
}
+
+ @Test
+ public void assertCreateDeletedEvent() {
+ Optional<GovernanceEvent> actual = globalLockChangedListener.createEvent(new DataChangedEvent("/lock/glock/lock-test-id", "", Type.DELETED));
+ assertTrue(actual.isPresent());
+ assertTrue(actual.get() instanceof GlobalLockReleasedEvent);
+ }
}
diff --git a/shardingsphere-jdbc/shardingsphere-jdbc-core/src/main/java/org/apache/shardingsphere/driver/executor/DriverJDBCExecutor.java b/shardingsphere-jdbc/shardingsphere-jdbc-core/src/main/java/org/apache/shardingsphere/driver/executor/DriverJDBCExecutor.java
index 356ec31..9f41d32 100644
--- a/shardingsphere-jdbc/shardingsphere-jdbc-core/src/main/java/org/apache/shardingsphere/driver/executor/DriverJDBCExecutor.java
+++ b/shardingsphere-jdbc/shardingsphere-jdbc-core/src/main/java/org/apache/shardingsphere/driver/executor/DriverJDBCExecutor.java
@@ -125,12 +125,13 @@ public final class DriverJDBCExecutor {
try {
locked = tryGlobalLock(sqlStatement, metaDataContexts.getProps().<Long>getValue(ConfigurationPropertyKey.LOCK_WAIT_TIMEOUT_MILLISECONDS));
results = jdbcExecutor.execute(executionGroups, callback);
- refreshSchema(metaDataContexts.getDefaultMetaData(), sqlStatement, routeUnits);
- } finally {
+ } catch (final SQLException ex) {
if (locked) {
releaseGlobalLock();
}
+ throw ex;
}
+ refreshSchema(metaDataContexts.getDefaultMetaData(), sqlStatement, routeUnits);
return results;
}
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 73fe0d5..432888c 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
@@ -102,18 +102,19 @@ public final class DatabaseCommunicationEngine {
if (executionContext.getExecutionUnits().isEmpty()) {
return new UpdateResponseHeader(executionContext.getSqlStatementContext().getSqlStatement());
}
+ proxySQLExecutor.checkExecutePrerequisites(executionContext);
boolean locked = false;
Collection<ExecuteResult> executeResults;
try {
locked = tryGlobalLock(executionContext, ProxyContext.getInstance().getMetaDataContexts().getProps().<Long>getValue(ConfigurationPropertyKey.LOCK_WAIT_TIMEOUT_MILLISECONDS));
- proxySQLExecutor.checkExecutePrerequisites(executionContext);
executeResults = proxySQLExecutor.execute(executionContext);
- refreshMetadata(executionContext);
- } finally {
+ } catch (final SQLException ex) {
if (locked) {
releaseGlobalLock();
}
+ throw ex;
}
+ refreshMetadata(executionContext);
ExecuteResult executeResultSample = executeResults.iterator().next();
return executeResultSample instanceof QueryResult
? processExecuteQuery(executionContext, executeResults.stream().map(each -> (QueryResult) each).collect(Collectors.toList()), (QueryResult) executeResultSample)