You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@aurora.apache.org by wf...@apache.org on 2015/07/06 23:21:40 UTC
aurora git commit: Fix DB constraint violation when updating host
attributes.
Repository: aurora
Updated Branches:
refs/heads/master 318b40d71 -> 7a536a0c3
Fix DB constraint violation when updating host attributes.
Bugs closed: AURORA-1379
Reviewed at https://reviews.apache.org/r/36096/
Project: http://git-wip-us.apache.org/repos/asf/aurora/repo
Commit: http://git-wip-us.apache.org/repos/asf/aurora/commit/7a536a0c
Tree: http://git-wip-us.apache.org/repos/asf/aurora/tree/7a536a0c
Diff: http://git-wip-us.apache.org/repos/asf/aurora/diff/7a536a0c
Branch: refs/heads/master
Commit: 7a536a0c3cf13f7d19c1c5f8f5716fe6c15339fb
Parents: 318b40d
Author: Bill Farner <wf...@apache.org>
Authored: Mon Jul 6 14:21:18 2015 -0700
Committer: Bill Farner <wf...@apache.org>
Committed: Mon Jul 6 14:21:18 2015 -0700
----------------------------------------------------------------------
.../scheduler/storage/AttributeStore.java | 7 +++++
.../scheduler/storage/db/AttributeMapper.java | 20 ++++++++++--
.../scheduler/storage/db/DbAttributeStore.java | 33 +++++++++++---------
.../scheduler/storage/db/AttributeMapper.xml | 15 ++++++---
.../storage/db/DbAttributeStoreTest.java | 32 +++++++++++++++++++
5 files changed, 85 insertions(+), 22 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/aurora/blob/7a536a0c/src/main/java/org/apache/aurora/scheduler/storage/AttributeStore.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/storage/AttributeStore.java b/src/main/java/org/apache/aurora/scheduler/storage/AttributeStore.java
index eef758b..36bdc81 100644
--- a/src/main/java/org/apache/aurora/scheduler/storage/AttributeStore.java
+++ b/src/main/java/org/apache/aurora/scheduler/storage/AttributeStore.java
@@ -32,6 +32,13 @@ import org.apache.mesos.Protos;
public interface AttributeStore {
/**
* Fetches all host attributes given by the host.
+ * <p>
+ * TODO(wfarner): We need to transition this API to key off slave ID instead of host name, as
+ * the host is not guaranteed to be unique (it's possible, and sometimes desirable to have
+ * multiple slaves on a machine). The current API ripples down into storage, since the store
+ * is not in the position to dictate behavior when host names collide. Most callers seem to have
+ * sufficient context to use slave ID instead, with the exception of those related to host
+ * maintenance.
*
* @param host host name.
* @return attributes associated with {@code host}, if the host is known.
http://git-wip-us.apache.org/repos/asf/aurora/blob/7a536a0c/src/main/java/org/apache/aurora/scheduler/storage/db/AttributeMapper.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/storage/db/AttributeMapper.java b/src/main/java/org/apache/aurora/scheduler/storage/db/AttributeMapper.java
index 3763f4d..a454887 100644
--- a/src/main/java/org/apache/aurora/scheduler/storage/db/AttributeMapper.java
+++ b/src/main/java/org/apache/aurora/scheduler/storage/db/AttributeMapper.java
@@ -18,7 +18,9 @@ import java.util.List;
import javax.annotation.Nullable;
import org.apache.aurora.gen.HostAttributes;
+import org.apache.aurora.gen.MaintenanceMode;
import org.apache.aurora.scheduler.storage.entities.IHostAttributes;
+import org.apache.ibatis.annotations.Param;
/**
* MyBatis mapper interface for Attribute.xml.
@@ -34,9 +36,21 @@ interface AttributeMapper {
/**
* Deletes all attributes and attribute values associated with a slave.
*
- * @param slaveId Slave ID to delete associated values from.
+ * @param host Host to delete associated values from.
*/
- void deleteAttributesAndValues(String slaveId);
+ void deleteAttributeValues(@Param("host") String host);
+
+ /**
+ * Updates the mode and slave ID associated with a host.
+ *
+ * @param host Host to update.
+ * @param mode New host maintenance mode.
+ * @param slaveId New host slave ID.
+ */
+ void updateHostModeAndSlaveId(
+ @Param("host") String host,
+ @Param("mode") MaintenanceMode mode,
+ @Param("slaveId") String slaveId);
/**
* Inserts values in {@link IHostAttributes#getAttributes()}, associating them with
@@ -53,7 +67,7 @@ interface AttributeMapper {
* @return Attributes associated with {@code host}, or {@code null} if no association exists.
*/
@Nullable
- HostAttributes select(String host);
+ HostAttributes select(@Param("host") String host);
/**
* Retrieves all stored host attributes.
http://git-wip-us.apache.org/repos/asf/aurora/blob/7a536a0c/src/main/java/org/apache/aurora/scheduler/storage/db/DbAttributeStore.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/storage/db/DbAttributeStore.java b/src/main/java/org/apache/aurora/scheduler/storage/db/DbAttributeStore.java
index 5f1cd2b..1b274c8 100644
--- a/src/main/java/org/apache/aurora/scheduler/storage/db/DbAttributeStore.java
+++ b/src/main/java/org/apache/aurora/scheduler/storage/db/DbAttributeStore.java
@@ -53,13 +53,29 @@ class DbAttributeStore implements AttributeStore.Mutable {
checkArgument(hostAttributes.isSetAttributes());
checkArgument(hostAttributes.isSetMode());
+ if (Iterables.any(hostAttributes.getAttributes(), EMPTY_VALUES)) {
+ throw new IllegalArgumentException(
+ "Host attributes contains empty values: " + hostAttributes);
+ }
+
Optional<IHostAttributes> existing = getHostAttributes(hostAttributes.getHost());
if (existing.equals(Optional.of(hostAttributes))) {
return false;
+ } else if (existing.isPresent()) {
+ mapper.updateHostModeAndSlaveId(
+ hostAttributes.getHost(),
+ hostAttributes.getMode(),
+ hostAttributes.getSlaveId());
} else {
- merge(hostAttributes);
- return true;
+ mapper.insert(hostAttributes);
}
+
+ mapper.deleteAttributeValues(hostAttributes.getHost());
+ if (!hostAttributes.getAttributes().isEmpty()) {
+ mapper.insertAttributeValues(hostAttributes);
+ }
+
+ return true;
}
private static final Predicate<IAttribute> EMPTY_VALUES = new Predicate<IAttribute>() {
@@ -69,19 +85,6 @@ class DbAttributeStore implements AttributeStore.Mutable {
}
};
- private void merge(IHostAttributes hostAttributes) {
- if (Iterables.any(hostAttributes.getAttributes(), EMPTY_VALUES)) {
- throw new IllegalArgumentException(
- "Host attributes contains empty values: " + hostAttributes);
- }
-
- mapper.deleteAttributesAndValues(hostAttributes.getHost());
- mapper.insert(hostAttributes);
- if (!hostAttributes.getAttributes().isEmpty()) {
- mapper.insertAttributeValues(hostAttributes);
- }
- }
-
@Timed("attribute_store_fetch_one")
@Override
public Optional<IHostAttributes> getHostAttributes(String host) {
http://git-wip-us.apache.org/repos/asf/aurora/blob/7a536a0c/src/main/resources/org/apache/aurora/scheduler/storage/db/AttributeMapper.xml
----------------------------------------------------------------------
diff --git a/src/main/resources/org/apache/aurora/scheduler/storage/db/AttributeMapper.xml b/src/main/resources/org/apache/aurora/scheduler/storage/db/AttributeMapper.xml
index d36b42a..41519de 100644
--- a/src/main/resources/org/apache/aurora/scheduler/storage/db/AttributeMapper.xml
+++ b/src/main/resources/org/apache/aurora/scheduler/storage/db/AttributeMapper.xml
@@ -31,12 +31,19 @@
</foreach>
</insert>
- <delete id="deleteAttributesAndValues">
+ <delete id="deleteAttributeValues">
<!-- This assumes the schema enables cascading deletes in the values table. -->
- DELETE FROM host_attributes
- WHERE host = #{id}
+ DELETE FROM host_attribute_values
+ WHERE host_attribute_id IN (SELECT id FROM host_attributes WHERE host = #{host})
</delete>
+ <update id="updateHostModeAndSlaveId">
+ UPDATE host_attributes SET
+ mode = #{mode, typeHandler=org.apache.aurora.scheduler.storage.db.typehandlers.MaintenanceModeTypeHandler},
+ slave_id = #{slaveId}
+ WHERE host = #{host}
+ </update>
+
<resultMap id="hostAttributeResultMap" type="org.apache.aurora.gen.HostAttributes">
<id column="a_id" />
<result property="mode"
@@ -69,7 +76,7 @@
<select id="select" resultMap="hostAttributeResultMap">
<include refid="unscoped_select"/>
- WHERE host = #{id}
+ WHERE host = #{host}
</select>
<select id="selectAll" resultMap="hostAttributeResultMap">
http://git-wip-us.apache.org/repos/asf/aurora/blob/7a536a0c/src/test/java/org/apache/aurora/scheduler/storage/db/DbAttributeStoreTest.java
----------------------------------------------------------------------
diff --git a/src/test/java/org/apache/aurora/scheduler/storage/db/DbAttributeStoreTest.java b/src/test/java/org/apache/aurora/scheduler/storage/db/DbAttributeStoreTest.java
index e19d15d..a7de023 100644
--- a/src/test/java/org/apache/aurora/scheduler/storage/db/DbAttributeStoreTest.java
+++ b/src/test/java/org/apache/aurora/scheduler/storage/db/DbAttributeStoreTest.java
@@ -22,12 +22,16 @@ import com.google.common.collect.ImmutableSet;
import org.apache.aurora.gen.Attribute;
import org.apache.aurora.gen.HostAttributes;
import org.apache.aurora.gen.MaintenanceMode;
+import org.apache.aurora.gen.ScheduledTask;
+import org.apache.aurora.scheduler.base.JobKeys;
+import org.apache.aurora.scheduler.base.TaskTestUtil;
import org.apache.aurora.scheduler.storage.Storage;
import org.apache.aurora.scheduler.storage.Storage.MutableStoreProvider;
import org.apache.aurora.scheduler.storage.Storage.MutateWork;
import org.apache.aurora.scheduler.storage.Storage.StoreProvider;
import org.apache.aurora.scheduler.storage.Storage.Work;
import org.apache.aurora.scheduler.storage.entities.IHostAttributes;
+import org.apache.aurora.scheduler.storage.entities.IScheduledTask;
import org.junit.Before;
import org.junit.Test;
@@ -130,6 +134,34 @@ public class DbAttributeStoreTest {
assertEquals(Optional.of(updated), read(HOST_A));
}
+ @Test
+ public void testUpdateAttributesWithRelations() {
+ // Test for regression of AURORA-1379, where host attribute mutation performed a delete,
+ // violating foreign key constraints.
+ insert(HOST_A_ATTRS);
+
+ ScheduledTask builder = TaskTestUtil.makeTask("a", JobKeys.from("role", "env", "job"))
+ .newBuilder();
+ builder.getAssignedTask()
+ .setSlaveHost(HOST_A_ATTRS.getHost())
+ .setSlaveId(HOST_A_ATTRS.getSlaveId());
+ final IScheduledTask taskA = IScheduledTask.build(builder);
+
+ storage.write(new MutateWork.NoResult.Quiet() {
+ @Override
+ protected void execute(MutableStoreProvider storeProvider) {
+ storeProvider.getUnsafeTaskStore().saveTasks(ImmutableSet.of(taskA));
+ }
+ });
+
+ HostAttributes attributeBuilder = HOST_A_ATTRS.newBuilder()
+ .setMode(MaintenanceMode.DRAINED);
+ attributeBuilder.addToAttributes(new Attribute("newAttr", ImmutableSet.of("a", "b")));
+ IHostAttributes hostAUpdated = IHostAttributes.build(attributeBuilder);
+ insert(hostAUpdated);
+ assertEquals(Optional.of(hostAUpdated), read(HOST_A));
+ }
+
private void insert(final IHostAttributes attributes) {
storage.write(new MutateWork.NoResult.Quiet() {
@Override