From 646c787aff71cba3f3a07c48a3ed7de019e43f4a Mon Sep 17 00:00:00 2001
From: amyangfei <amyangfei@gmail.com>
Date: Fri, 28 Jun 2019 10:19:46 +0800
Subject: [PATCH] address comments

---
 syncer/sharding-meta/shardmeta.go      | 14 ++++++++++++++
 syncer/sharding-meta/shardmeta_test.go |  7 +++++++
 syncer/sharding_group.go               |  4 ++--
 3 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/syncer/sharding-meta/shardmeta.go b/syncer/sharding-meta/shardmeta.go
index 5054349f1d..5259c2a593 100644
--- a/syncer/sharding-meta/shardmeta.go
+++ b/syncer/sharding-meta/shardmeta.go
@@ -79,6 +79,7 @@ func (seq *ShardingSequence) String() string {
 
 // ShardingMeta stores sharding ddl sequence
 // including global sequence and each source's own sequence
+// NOTE: sharding meta is not thread safe, it must be used in thread safe context
 type ShardingMeta struct {
 	activeIdx int                          // the first unsynced DDL index
 	global    *ShardingSequence            // merged sharding sequence of all source tables
@@ -178,6 +179,19 @@ func (meta *ShardingMeta) AddItem(item *DDLItem) (active bool, err error) {
 	return index == meta.activeIdx, nil
 }
 
+// GetGlobalItems returns activeDDL in global sequence
+func (meta *ShardingMeta) GetGlobalActiveDDL() *DDLItem {
+	if meta.activeIdx < len(meta.global.Items) {
+		return meta.global.Items[meta.activeIdx]
+	}
+	return nil
+}
+
+// GetGlobalItems returns global DDLItems
+func (meta *ShardingMeta) GetGlobalItems() []*DDLItem {
+	return meta.global.Items
+}
+
 // GetActiveDDLItem returns the source table's active DDLItem
 // if in DDL unsynced procedure, the active DDLItem means the syncing DDL
 // if in re-sync procedure, the active DDLItem means the next syncing DDL in DDL syncing sequence, may be nil
diff --git a/syncer/sharding-meta/shardmeta_test.go b/syncer/sharding-meta/shardmeta_test.go
index 933f5f33e3..5812e4f671 100644
--- a/syncer/sharding-meta/shardmeta_test.go
+++ b/syncer/sharding-meta/shardmeta_test.go
@@ -70,6 +70,8 @@ func (t *testShardMetaSuite) TestShardingMeta(c *check.C) {
 		}
 	}
 
+	c.Assert(meta.GetGlobalItems(), check.DeepEquals, []*DDLItem{items[0], items[1], items[2]})
+	c.Assert(meta.GetGlobalActiveDDL(), check.DeepEquals, items[0])
 	c.Assert(meta.GetActiveDDLItem(table1), check.DeepEquals, items[0])
 	c.Assert(meta.GetActiveDDLItem(table2), check.DeepEquals, items[3])
 	c.Assert(meta.GetActiveDDLItem(table3), check.DeepEquals, items[6])
@@ -81,6 +83,7 @@ func (t *testShardMetaSuite) TestShardingMeta(c *check.C) {
 	// find synced in shrading group, and call ShardingMeta.ResolveShardingDDL
 	c.Assert(meta.ResolveShardingDDL(), check.IsFalse)
 
+	c.Assert(meta.GetGlobalActiveDDL(), check.DeepEquals, items[1])
 	c.Assert(meta.GetActiveDDLItem(table1), check.DeepEquals, items[1])
 	c.Assert(meta.GetActiveDDLItem(table2), check.DeepEquals, items[4])
 	c.Assert(meta.GetActiveDDLItem(table3), check.IsNil)
@@ -114,6 +117,7 @@ func (t *testShardMetaSuite) TestShardingMeta(c *check.C) {
 		}
 	}
 
+	c.Assert(meta.GetGlobalActiveDDL(), check.DeepEquals, items[1])
 	c.Assert(meta.GetActiveDDLItem(table1), check.DeepEquals, items[1])
 	c.Assert(meta.GetActiveDDLItem(table2), check.DeepEquals, items[4])
 	c.Assert(meta.GetActiveDDLItem(table3), check.DeepEquals, items[7])
@@ -125,6 +129,7 @@ func (t *testShardMetaSuite) TestShardingMeta(c *check.C) {
 	// find synced in shrading group, and call ShardingMeta.ResolveShardingDDL
 	c.Assert(meta.ResolveShardingDDL(), check.IsFalse)
 
+	c.Assert(meta.GetGlobalActiveDDL(), check.DeepEquals, items[2])
 	c.Assert(meta.GetActiveDDLItem(table1), check.DeepEquals, items[2])
 	c.Assert(meta.GetActiveDDLItem(table2), check.DeepEquals, items[5])
 	c.Assert(meta.GetActiveDDLItem(table3), check.IsNil)
@@ -157,6 +162,7 @@ func (t *testShardMetaSuite) TestShardingMeta(c *check.C) {
 			c.Assert(active, check.IsFalse)
 		}
 	}
+	c.Assert(meta.GetGlobalActiveDDL(), check.DeepEquals, items[2])
 	c.Assert(meta.GetActiveDDLItem(table1), check.DeepEquals, items[2])
 	c.Assert(meta.GetActiveDDLItem(table2), check.DeepEquals, items[5])
 	c.Assert(meta.GetActiveDDLItem(table3), check.DeepEquals, items[8])
@@ -168,6 +174,7 @@ func (t *testShardMetaSuite) TestShardingMeta(c *check.C) {
 	// find synced in shrading group, and call ShardingMeta.ResolveShardingDDL
 	c.Assert(meta.ResolveShardingDDL(), check.IsTrue)
 
+	c.Assert(meta.GetGlobalActiveDDL(), check.IsNil)
 	c.Assert(meta.GetActiveDDLItem(table1), check.IsNil)
 	c.Assert(meta.GetActiveDDLItem(table2), check.IsNil)
 	c.Assert(meta.GetActiveDDLItem(table3), check.IsNil)
diff --git a/syncer/sharding_group.go b/syncer/sharding_group.go
index ac432aeb8e..99b1c436c3 100644
--- a/syncer/sharding_group.go
+++ b/syncer/sharding_group.go
@@ -137,7 +137,7 @@ func (sg *ShardingGroup) Merge(sources []string) (bool, bool, int, error) {
 
 	// NOTE: we don't support add shard table when in sequence sharding
 	if sg.meta.InSequenceSharding() {
-		return true, sg.remain <= 0, sg.remain, errors.NotSupportedf("in sequence sharding, can't add table")
+		return true, sg.remain <= 0, sg.remain, errors.NotSupportedf("in sequence sharding, can't add table, activeDDL: %s, sharding sequence: %s", sg.meta.GetGlobalActiveDDL(), sg.meta.GetGlobalItems())
 	}
 
 	for _, source := range sources {
@@ -162,7 +162,7 @@ func (sg *ShardingGroup) Leave(sources []string) error {
 
 	// NOTE: if group is in sequence sharding, we can't do drop (DROP DATABASE / TABLE)
 	if sg.meta.InSequenceSharding() {
-		return errors.NotSupportedf("group's sharding DDL %v is un-resolved, try drop sources %v", sg.ddls, sources)
+		return errors.NotSupportedf("in sequence sharding, try drop sources %v, activeDDL: %s, sharding sequence: %s", sources, sg.meta.GetGlobalActiveDDL(), sg.meta.GetGlobalItems())
 	}
 
 	for _, source := range sources {