Skip to content

Commit da2e06c

Browse files
dstreevdstreev
committed
[--sync not dropping table on right when left is missing.](#189)
1 parent 4d2dd74 commit da2e06c

32 files changed

+1445
-247
lines changed

‎Writerside/topics/Release-Notes.md‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,15 +10,18 @@ The latest set of enhancement requests can be found [here](https://github.com/cl
1010

1111
If there is something you'd like to see, add a new issue [here](https://github.com/cloudera-labs/hms-mirror/issues)
1212

13-
## 3.0.0.0
13+
## 3.0.0.1
1414

1515
This release is based on the 2.3.1.5 release and includes all the features and bug fixes from that release.
1616

1717
This is a Security and CVE release that has upgrading all dependencies to the latest possible versions to eliminate
1818
as many of the community CVEs as possible. This also required us to upgrade the minimum JDK version to 17.
1919

20-
***What's New**
21-
- JDK 17 Minimum Version Requirement.
20+
**What's New**
21+
- JDK 17 Minimum Version Requirement. Addresses dependencies with CVE issues.
22+
23+
**Bug Fixes**
24+
[--sync not dropping table on right when left is missing.](https://github.com/cloudera-labs/hms-mirror/issues/189)
2225

2326
## 2.3.1.5
2427

‎pom.xml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@
2828

2929
<groupId>com.cloudera.utils.hadoop</groupId>
3030
<artifactId>hms-mirror</artifactId>
31-
<version>3.0.0.0</version>
31+
<version>3.0.0.1</version>
3232
<packaging>jar</packaging>
3333

3434
<name>hms-mirror</name>

‎src/main/java/com/cloudera/utils/hms/mirror/MessageCode.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -190,6 +190,7 @@ public enum MessageCode {
190190
SAME_CLUSTER_COPY_WITHOUT_DBPR("You must specify `-dbp` or `-dbr` when the cluster configurations use the same storage."),
191191
SAME_CLUSTER_COPY_WITHOUT_RDL("You must specify `-rdl` when the cluster configurations use the same storage."),
192192
SCHEMA_EXISTS_NO_ACTION("Schema exists already and matches. No action necessary"),
193+
SCHEMA_EXISTS_TARGET_MISMATCH("Schema exists on the target, but not on the source."),
193194
SCHEMA_EXISTS_NO_ACTION_DATA("Schema exists already. Drop it and " +
194195
"try again or add `--sync` to OVERWRITE current tables data."),
195196
SCHEMA_EXISTS_NOT_MATCH_WITH_PURGE("Schema exists AND DOESN'T match. But the 'RIGHT' table is has a PURGE option set. " +

‎src/main/java/com/cloudera/utils/hms/mirror/cli/config/CliInit.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -318,14 +318,15 @@ public CommandLineRunner conversionPostProcessing(HmsMirrorConfig builtConfig,
318318
for (DBMirror dbMirror : conversion.getDatabases().values()) {
319319
String database = dbMirror.getName();
320320
for (TableMirror tableMirror : dbMirror.getTableMirrors().values()) {
321+
EnvironmentTable et = tableMirror.getEnvironmentTable(Environment.LEFT);
321322
String tableName = tableMirror.getName();
322323
if (config.isDatabaseOnly()) {
323324
// Only work with the database.
324325
tableMirror.setRemove(true);
325-
} else if (TableUtils.isACID(tableMirror.getEnvironmentTable(Environment.LEFT))
326+
} else if (TableUtils.isACID(et)
326327
&& !config.getMigrateACID().isOn()) {
327328
tableMirror.setRemove(true);
328-
} else if (!TableUtils.isACID(tableMirror.getEnvironmentTable(Environment.LEFT))
329+
} else if (!TableUtils.isACID(et)
329330
&& config.getMigrateACID().isOnly()) {
330331
tableMirror.setRemove(true);
331332
} else {

‎src/main/java/com/cloudera/utils/hms/mirror/datastrategy/CommonDataStrategy.java‎

Lines changed: 57 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -76,75 +76,77 @@ public Boolean buildOutDefinition(TableMirror tableMirror) throws RequiredConfig
7676

7777
copySpec = new CopySpec(tableMirror, Environment.LEFT, Environment.RIGHT);
7878
// Can't LINK ACID tables.
79-
if (TableUtils.isHiveNative(let) && !TableUtils.isACID(let)) {
80-
// For COMMON, we're assuming the namespace is used by 'both' so we don't change anything..
81-
copySpec.setReplaceLocation(Boolean.FALSE);
82-
if (hmsMirrorConfig.convertManaged())
83-
copySpec.setUpgrade(Boolean.TRUE);
84-
// COMMON owns the data unless readonly specified.
85-
if (!hmsMirrorConfig.isReadOnly())
86-
copySpec.setTakeOwnership(Boolean.TRUE);
87-
if (hmsMirrorConfig.isNoPurge())
88-
copySpec.setTakeOwnership(Boolean.FALSE);
79+
if (let.isExists()) {
80+
if (TableUtils.isHiveNative(let) && !TableUtils.isACID(let)) {
81+
// For COMMON, we're assuming the namespace is used by 'both' so we don't change anything..
82+
copySpec.setReplaceLocation(Boolean.FALSE);
83+
if (hmsMirrorConfig.convertManaged())
84+
copySpec.setUpgrade(Boolean.TRUE);
85+
// COMMON owns the data unless readonly specified.
86+
if (!hmsMirrorConfig.isReadOnly())
87+
copySpec.setTakeOwnership(Boolean.TRUE);
88+
if (hmsMirrorConfig.isNoPurge())
89+
copySpec.setTakeOwnership(Boolean.FALSE);
8990

90-
if (hmsMirrorConfig.isSync()) {
91-
// We assume that the 'definitions' are only there if the
92-
// table exists.
93-
if (!let.isExists() && ret.isExists()) {
94-
// If left is empty and right is not, DROP RIGHT.
95-
ret.addIssue("Schema doesn't exist in 'source'. Will be DROPPED.");
96-
ret.setCreateStrategy(CreateStrategy.DROP);
97-
} else if (let.isExists() && !ret.isExists()) {
98-
// If left is defined and right is not, CREATE RIGHT.
99-
ret.addIssue("Schema missing, will be CREATED");
100-
ret.setCreateStrategy(CreateStrategy.CREATE);
101-
} else if (let.isExists() && ret.isExists()) {
102-
// If left and right, check schema change and replace if necessary.
103-
// Compare Schemas.
104-
if (tableMirror.schemasEqual(Environment.LEFT, Environment.RIGHT)) {
91+
if (hmsMirrorConfig.isSync()) {
92+
// We assume that the 'definitions' are only there if the
93+
// table exists.
94+
if (let.isExists() && !ret.isExists()) {
95+
// If left is defined and right is not, CREATE RIGHT.
96+
ret.addIssue("Schema missing, will be CREATED");
97+
ret.setCreateStrategy(CreateStrategy.CREATE);
98+
} else if (let.isExists() && ret.isExists()) {
99+
// If left and right, check schema change and replace if necessary.
100+
// Compare Schemas.
101+
if (tableMirror.schemasEqual(Environment.LEFT, Environment.RIGHT)) {
102+
ret.addIssue(SCHEMA_EXISTS_NO_ACTION_DATA.getDesc());
103+
ret.addSql(SKIPPED.getDesc(), "-- " + SCHEMA_EXISTS_NO_ACTION_DATA.getDesc());
104+
ret.setCreateStrategy(CreateStrategy.LEAVE);
105+
String msg = MessageFormat.format(TABLE_ISSUE.getDesc(), tableMirror.getParent().getName(), tableMirror.getName(),
106+
SCHEMA_EXISTS_NO_ACTION_DATA.getDesc());
107+
log.error(msg);
108+
} else {
109+
if (TableUtils.isExternalPurge(ret)) {
110+
ret.addIssue(SCHEMA_EXISTS_NOT_MATCH_WITH_PURGE.getDesc());
111+
ret.addSql(SKIPPED.getDesc(), "-- " + SCHEMA_EXISTS_NOT_MATCH_WITH_PURGE.getDesc());
112+
ret.setCreateStrategy(CreateStrategy.LEAVE);
113+
String msg = MessageFormat.format(TABLE_ISSUE.getDesc(), tableMirror.getParent().getName(), tableMirror.getName(),
114+
SCHEMA_EXISTS_NOT_MATCH_WITH_PURGE.getDesc());
115+
log.error(msg);
116+
return Boolean.FALSE;
117+
} else {
118+
ret.addIssue("Schema exists AND DOESN'T match. It will be REPLACED (DROPPED and RECREATED).");
119+
ret.setCreateStrategy(CreateStrategy.REPLACE);
120+
}
121+
}
122+
}
123+
// With sync, don't own data.
124+
copySpec.setTakeOwnership(Boolean.FALSE);
125+
} else {
126+
if (ret.isExists()) {
127+
// Already exists, no action.
105128
ret.addIssue(SCHEMA_EXISTS_NO_ACTION_DATA.getDesc());
106129
ret.addSql(SKIPPED.getDesc(), "-- " + SCHEMA_EXISTS_NO_ACTION_DATA.getDesc());
107130
ret.setCreateStrategy(CreateStrategy.LEAVE);
108131
String msg = MessageFormat.format(TABLE_ISSUE.getDesc(), tableMirror.getParent().getName(), tableMirror.getName(),
109132
SCHEMA_EXISTS_NO_ACTION_DATA.getDesc());
110133
log.error(msg);
134+
return Boolean.FALSE;
111135
} else {
112-
if (TableUtils.isExternalPurge(ret)) {
113-
ret.addIssue(SCHEMA_EXISTS_NOT_MATCH_WITH_PURGE.getDesc());
114-
ret.addSql(SKIPPED.getDesc(), "-- " + SCHEMA_EXISTS_NOT_MATCH_WITH_PURGE.getDesc());
115-
ret.setCreateStrategy(CreateStrategy.LEAVE);
116-
String msg = MessageFormat.format(TABLE_ISSUE.getDesc(), tableMirror.getParent().getName(), tableMirror.getName(),
117-
SCHEMA_EXISTS_NOT_MATCH_WITH_PURGE.getDesc());
118-
log.error(msg);
119-
return Boolean.FALSE;
120-
} else {
121-
ret.addIssue("Schema exists AND DOESN'T match. It will be REPLACED (DROPPED and RECREATED).");
122-
ret.setCreateStrategy(CreateStrategy.REPLACE);
123-
}
136+
ret.addIssue("Schema will be created");
137+
ret.setCreateStrategy(CreateStrategy.CREATE);
124138
}
125139
}
126-
// With sync, don't own data.
127-
copySpec.setTakeOwnership(Boolean.FALSE);
140+
// Rebuild Target from Source.
141+
rtn = buildTableSchema(copySpec);
128142
} else {
129-
if (ret.isExists()) {
130-
// Already exists, no action.
131-
ret.addIssue(SCHEMA_EXISTS_NO_ACTION_DATA.getDesc());
132-
ret.addSql(SKIPPED.getDesc(), "-- " + SCHEMA_EXISTS_NO_ACTION_DATA.getDesc());
133-
ret.setCreateStrategy(CreateStrategy.LEAVE);
134-
String msg = MessageFormat.format(TABLE_ISSUE.getDesc(), tableMirror.getParent().getName(), tableMirror.getName(),
135-
SCHEMA_EXISTS_NO_ACTION_DATA.getDesc());
136-
log.error(msg);
137-
return Boolean.FALSE;
138-
} else {
139-
ret.addIssue("Schema will be created");
140-
ret.setCreateStrategy(CreateStrategy.CREATE);
141-
}
143+
let.addIssue("Can't use COMMON for ACID tables");
144+
ret.setCreateStrategy(CreateStrategy.NOTHING);
142145
}
143-
// Rebuild Target from Source.
144-
rtn = buildTableSchema(copySpec);
145146
} else {
146-
let.addIssue("Can't use COMMON for ACID tables");
147+
let.addIssue(SCHEMA_EXISTS_TARGET_MISMATCH.getDesc());
147148
ret.setCreateStrategy(CreateStrategy.NOTHING);
149+
rtn = Boolean.TRUE;
148150
}
149151
return rtn;
150152
}

‎src/main/java/com/cloudera/utils/hms/mirror/datastrategy/DataStrategyBase.java‎

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -484,14 +484,6 @@ public Boolean buildTableSchema(CopySpec copySpec) throws RequiredConfigurationE
484484
}
485485
}
486486

487-
// Add props to definition.
488-
if (tableMirror.whereTherePropsAdded(copySpec.getTarget())) {
489-
Set<String> keys = target.getAddProperties().keySet();
490-
for (String key : keys) {
491-
TableUtils.upsertTblProperty(key, target.getAddProperties().get(key), target);
492-
}
493-
}
494-
495487
if (!copySpec.isTakeOwnership() && config.getDataStrategy() != DataStrategyEnum.STORAGE_MIGRATION) {
496488
TableUtils.removeTblProperty(EXTERNAL_TABLE_PURGE, target);
497489
}
@@ -504,6 +496,14 @@ public Boolean buildTableSchema(CopySpec copySpec) throws RequiredConfigurationE
504496
TableUtils.removeTblProperty(BUCKETING_VERSION, target);
505497
}
506498

499+
// Add props to definition.
500+
if (tableMirror.whereTherePropsAdded(copySpec.getTarget())) {
501+
Set<String> keys = target.getAddProperties().keySet();
502+
for (String key : keys) {
503+
TableUtils.upsertTblProperty(key, target.getAddProperties().get(key), target);
504+
}
505+
}
506+
507507
} else if (TableUtils.isView(target)) {
508508
source.addIssue("This is a VIEW. It will be translated AS-IS. View transitions will NOT honor " +
509509
"target db name changes For example: `-dbp`. VIEW creation depends on the referenced tables existing FIRST. " +
@@ -515,6 +515,8 @@ public Boolean buildTableSchema(CopySpec copySpec) throws RequiredConfigurationE
515515

516516
TableUtils.fixTableDefinition(target);
517517
}
518+
// Set this so that checks can be done on the target.
519+
target.setExists(Boolean.TRUE);
518520
} catch (MismatchException e) {
519521
log.error("Error building table schema: {}", e.getMessage(), e);
520522
source.addError("Error building table schema: " + e.getMessage());

‎src/main/java/com/cloudera/utils/hms/mirror/datastrategy/ExportImportDataStrategy.java‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -289,13 +289,17 @@ public Boolean build(TableMirror tableMirror) {
289289
EnvironmentTable let = tableMirror.getEnvironmentTable(Environment.LEFT);
290290
EnvironmentTable ret = tableMirror.getEnvironmentTable(Environment.RIGHT);
291291
if (ret.isExists()) {
292-
if (!hmsMirrorConfig.isSync()) {
292+
if (!hmsMirrorConfig.isSync() && let.isExists()) {
293293
let.addIssue(MessageCode.SCHEMA_EXISTS_NO_ACTION_DATA.getDesc());
294294
let.addSql(SKIPPED.getDesc(), "-- " + SCHEMA_EXISTS_NO_ACTION_DATA.getDesc());
295295
String msg = MessageFormat.format(TABLE_ISSUE.getDesc(), tableMirror.getParent().getName(), tableMirror.getName(),
296296
SCHEMA_EXISTS_NO_ACTION_DATA.getDesc());
297297
log.error(msg);
298298
return Boolean.FALSE;
299+
} else {
300+
ret.addIssue(SCHEMA_EXISTS_TARGET_MISMATCH.getDesc());
301+
ret.setCreateStrategy(CreateStrategy.LEAVE);
302+
return Boolean.TRUE;
299303
}
300304
}
301305

‎src/main/java/com/cloudera/utils/hms/mirror/datastrategy/IntermediateDataStrategy.java‎

Lines changed: 21 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -81,21 +81,28 @@ public Boolean buildOutDefinition(TableMirror tableMirror) throws RequiredConfig
8181
CopySpec rightSpec = new CopySpec(tableMirror, Environment.LEFT, Environment.RIGHT);
8282

8383
if (ret.isExists()) {
84-
if (!TableUtils.isACID(ret) && hmsMirrorConfig.getCluster(Environment.RIGHT).isCreateIfNotExists() && hmsMirrorConfig.isSync()) {
85-
ret.addIssue(CINE_WITH_EXIST.getDesc());
86-
ret.setCreateStrategy(CreateStrategy.CREATE);
87-
} else if (TableUtils.isACID(ret) && hmsMirrorConfig.isSync()) {
88-
ret.addIssue(SCHEMA_EXISTS_SYNC_ACID.getDesc());
89-
ret.setCreateStrategy(CreateStrategy.REPLACE);
84+
if (let.isExists()) {
85+
if (!TableUtils.isACID(ret) && hmsMirrorConfig.getCluster(Environment.RIGHT).isCreateIfNotExists() && hmsMirrorConfig.isSync()) {
86+
ret.addIssue(CINE_WITH_EXIST.getDesc());
87+
ret.setCreateStrategy(CreateStrategy.CREATE);
88+
} else if (TableUtils.isACID(ret) && hmsMirrorConfig.isSync()) {
89+
ret.addIssue(SCHEMA_EXISTS_SYNC_ACID.getDesc());
90+
ret.setCreateStrategy(CreateStrategy.REPLACE);
91+
} else {
92+
// Already exists, no action.
93+
ret.addIssue(SCHEMA_EXISTS_NO_ACTION_DATA.getDesc());
94+
ret.addSql(SKIPPED.getDesc(), "-- " + SCHEMA_EXISTS_NO_ACTION_DATA.getDesc());
95+
ret.setCreateStrategy(CreateStrategy.NOTHING);
96+
String msg = MessageFormat.format(TABLE_ISSUE.getDesc(), tableMirror.getParent().getName(), tableMirror.getName(),
97+
SCHEMA_EXISTS_NO_ACTION_DATA.getDesc());
98+
log.error(msg);
99+
return Boolean.FALSE;
100+
}
90101
} else {
91-
// Already exists, no action.
92-
ret.addIssue(SCHEMA_EXISTS_NO_ACTION_DATA.getDesc());
93-
ret.addSql(SKIPPED.getDesc(), "-- " + SCHEMA_EXISTS_NO_ACTION_DATA.getDesc());
94-
ret.setCreateStrategy(CreateStrategy.NOTHING);
95-
String msg = MessageFormat.format(TABLE_ISSUE.getDesc(), tableMirror.getParent().getName(), tableMirror.getName(),
96-
SCHEMA_EXISTS_NO_ACTION_DATA.getDesc());
97-
log.error(msg);
98-
return Boolean.FALSE;
102+
// Left doesn't exist, but the right does. Report and do nothing.
103+
ret.setCreateStrategy(CreateStrategy.LEAVE);
104+
ret.addIssue(SCHEMA_EXISTS_TARGET_MISMATCH.getDesc());
105+
return Boolean.TRUE;
99106
}
100107
} else {
101108
ret.setCreateStrategy(CreateStrategy.CREATE);

0 commit comments

Comments
 (0)