address comments

Signed-off-by: nolouch <nolouch@gmail.com>

ddl/placement/bundle.go

@@ -124,20 +124,27 @@ func NewBundleFromConstraintsOptions(options *model.PlacementSettings) (*Bundle,
 			rules = append(rules, rule)
 		}
 	}
+	labels, err := newLocationLabelsFromSurvivalPreferences(options.SurvivalPreferences)
+	if err != nil {
+		return nil, err
+	}
+	for _, rule := range rules {
+		rule.LocationLabels = labels
+	}
+	return &Bundle{Rules: rules}, nil
+}
 
-	survivalPreferenceStr := options.SurvivalPreferences
+// newLocationLabelsFromSurvivalPreferences will set the survival preference for all rules.
+func newLocationLabelsFromSurvivalPreferences(survivalPreferenceStr string) ([]string, error) {
 	if len(survivalPreferenceStr) > 0 {
 		labels := []string{}
 		err := yaml.UnmarshalStrict([]byte(survivalPreferenceStr), &labels)
 		if err != nil {
 			return nil, ErrInvalidSurvivalPreferenceFormat
 		}
-		for _, rule := range rules {
-			rule.LocationLabels = labels
-		}
+		return labels, nil
 	}
-
-	return &Bundle{Rules: rules}, nil
+	return nil, nil
 }
 
 // NewBundleFromSugarOptions will transform syntax sugar options into the bundle.
@@ -168,27 +175,16 @@ func NewBundleFromSugarOptions(options *model.PlacementSettings) (*Bundle, error
 
 	var rules []*Rule
 
-	survivalPreferenceStr := options.SurvivalPreferences
-	var locationLabels []string
-	if len(survivalPreferenceStr) > 0 {
-		labels := []string{}
-		err := yaml.UnmarshalStrict([]byte(survivalPreferenceStr), &labels)
-		if err != nil {
-			return nil, ErrInvalidConstraintsFormat
-		}
-
-		locationLabels = labels
-
+	locationLabels, err := newLocationLabelsFromSurvivalPreferences(options.SurvivalPreferences)
+	if err != nil {
+		return nil, err
 	}
 
 	// in case empty primaryRegion and regions, just return an empty bundle
-	// FIXME: @nolouch
 	if primaryRegion == "" && len(regions) == 0 {
 		rules = append(rules, NewRule(Voter, followers+1, NewConstraintsDirect()))
-		if len(survivalPreferenceStr) > 0 && len(locationLabels) > 0 {
-			for _, rule := range rules {
-				rule.LocationLabels = locationLabels
-			}
+		for _, rule := range rules {
+			rule.LocationLabels = locationLabels
 		}
 		return &Bundle{Rules: rules}, nil
 	}
@@ -227,10 +223,9 @@ func NewBundleFromSugarOptions(options *model.PlacementSettings) (*Bundle, error
 		}
 	}
 
-	if len(survivalPreferenceStr) > 0 && len(locationLabels) > 0 {
-		for _, rule := range rules {
-			rule.LocationLabels = locationLabels
-		}
+	// set location labels
+	for _, rule := range rules {
+		rule.LocationLabels = locationLabels
 	}
 
 	return &Bundle{Rules: rules}, nil

ddl/placement_policy_test.go

@@ -594,7 +594,7 @@ func TestCreateTableWithPlacementPolicy(t *testing.T) {
 		"CONSTRAINTS=\"[+disk=ssd]\" ")
 	tk.MustExec("create placement policy z " +
 		"FOLLOWERS=1 " +
-		"SURVIVAL_PREFERENCES=\"[zone, region]\"")
+		"SURVIVAL_PREFERENCES=\"[region, zone]\"")
 	tk.MustExec("create placement policy y " +
 		"FOLLOWERS=3 " +
 		"CONSTRAINTS=\"[+region=bj]\" ")
@@ -627,7 +627,7 @@ func TestCreateTableWithPlacementPolicy(t *testing.T) {
 	policyZ := testGetPolicyByName(t, tk.Session(), "z", true)
 	require.Equal(t, "z", policyZ.Name.L)
 	require.Equal(t, true, policyZ.ID != 0)
-	require.Equal(t, "[zone, region]", policyZ.SurvivalPreferences)
+	require.Equal(t, "[region, zone]", policyZ.SurvivalPreferences)
 
 	tbl := external.GetTableByName(t, tk, "test", "tt")
 	require.NotNil(t, tbl)

docs/design/2020-06-24-placement-rules-in-sql.md

@@ -413,6 +413,26 @@ If a table is imported when `tidb_placement_mode='IGNORE'`, and the placement po
 
 The default value for `tidb_placement_mode` is `STRICT`. The option is an enum, and in future we may add support for a `WARN` mode.
 
+
+#### survival preference
+
+Some important data may need to store multiple copies across availability zones, so as to have high disaster recovery survivability, such as region-level survivability, `SURVIVAL_PREFERENCES` can provide survivability preference settings.
+
+The following example sets a constraint that the data try to satisfy the Survival Preferences setting:
+
+``` sql
+CREATE PLACEMENT POLICY multiregion
+    follower=4
+    PRIMARY_REGION="region1"
+    SURVIVAL_PREFERENCES="[region, zone]";
+```
+
+For tables with this policy set, the data will first meet the survival goal of cross-region data isolation, and then ensure the survival goal of cross-zone data isolation.
+
+> **Note:**
+>
+> `SURVIVAL_PREFERENCES` is equivalent to `LOCATION_LABELS` in PD. For more information, please refer to [Replica scheduling by topology label](https://docs.pingcap.com/tidb/dev/schedule-replicas-by-topology-labels#schedule-replicas-by-topology-labels).
+
 #### Ambiguous and edge cases
 
 The following two policies are not identical:
@@ -491,6 +511,7 @@ In this case the default rules will apply to placement, and the output from `SHO
 - `FOLLOWER_CONSTRAINTS`
 - `LEARNER_CONSTRAINTS`
 - `PLACEMENT POLICY`
+- `SURVIVAL_PREFERENCE`
 
 For a more complex rule using partitions, consider the following example: