You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@hive.apache.org by "dengzhhu653 (via GitHub)" <gi...@apache.org> on 2026/10/16 09:25:16 UTC
[PR] HIVE-30194: HiveMetaTool drops silently without clear error message [hive]
dengzhhu653 opened a new pull request, #6887:
URL: https://github.com/apache/hive/pull/6887
<!--
Thanks for sending a pull request! Here are some tips for you:
1. If this is your first time, please read our contributor guidelines: https://cwiki.apache.org/confluence/display/Hive/HowToContribute
2. Ensure that you have created an issue on the Hive project JIRA: https://issues.apache.org/jira/projects/HIVE/summary
3. Ensure you have added or run the appropriate tests for your PR:
4. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP]HIVE-XXXXX: Your PR title ...'.
5. Be sure to keep the PR description updated to reflect all changes.
6. Please write your PR title to summarize what this PR proposes.
7. If possible, provide a concise example to reproduce the issue for a faster review.
-->
### What changes were proposed in this pull request?
- Reduce the memory footprint the ColumnDeduplicator needs;
- Show the exception at console;
- Add the deduplication marker to the table that has been processed, so the later re-runs of this command will ignore this table(no need to process again);
- A final stage to clean up the marker that has been added to the table.
<!--
Please clarify what changes you are proposing. The purpose of this section is to outline the changes and how this PR fixes the issue.
If possible, please consider writing useful notes for better and faster reviews in your PR. See the examples below.
1. If you refactor some codes with changing classes, showing the class hierarchy will help reviewers.
2. If you fix some SQL features, you can provide some references of other DBMSes.
3. If there is design documentation, please add the link.
4. If there is a discussion in the mailing list, please add the link.
-->
### Why are the changes needed?
- Make this command error-tolerance.
<!--
Please clarify why the changes are needed. For instance,
1. If you propose a new API, clarify the use case for a new API.
2. If you fix a bug, you can clarify why it is a bug.
-->
### Does this PR introduce _any_ user-facing change?
- Yes, this is a new feature
<!--
Note that it means *any* user-facing change including all aspects such as the documentation fix.
If yes, please clarify the previous behavior and the change this PR proposes - provide the console output, description, screenshot and/or a reproducible example to show the behavior difference if possible.
If possible, please also clarify if this is a user-facing change compared to the released Hive versions or within the unreleased branches such as master.
If no, write 'No'.
-->
### How was this patch tested?
- Unit tests
- Run the command backed by MySQL, Postgres
<!--
If tests were added, say they were added here. Please make sure to add some test cases that check the changes thoroughly including negative and positive cases if possible.
If it was tested in a way different from regular unit tests, please clarify how you tested step by step, ideally copy and paste-able, so that other reviewers can test and check, and descendants can verify in the future.
If tests were not added, please describe why they were not added and/or why it was difficult to add.
-->
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-30194: HiveMetaTool drops silently without clear error message [hive]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #6887:
URL: https://github.com/apache/hive/pull/6887#discussion_r4332921508
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/tools/ColumnDeduplicator.java:
##########
@@ -119,6 +132,39 @@ MetaToolObjectStore.DedupColumnsResult run(String catalogFilter, String dbFilter
return result;
}
+ public TableInfo getTable(long tableId) {
+ final MTable table;
+ try {
+ table = pm.getObjectById(MTable.class, new MTable.PK(tableId));
+ } catch (JDOObjectNotFoundException e) {
+ // This table might have been dropped since the beginning
+ return null;
+ }
+ Map<String, String> params = table.getParameters() == null ?
+ new HashMap<>() : table.getParameters();
+ if (clearMarker) {
+ if (!params.containsKey(COLUMN_DEDUPLICATOR_MARKER)) {
+ return null;
+ }
+ params.remove(COLUMN_DEDUPLICATOR_MARKER);
+ table.setParameters(params);
+ return null;
+ }
Review Comment:
`-dryRun` currently does not prevent marker deletion when `clearMarker` is enabled, because the `clearMarker` branch mutates table parameters unconditionally. This breaks the “no changes in dry run” contract; either disallow combining `-dryRun` with `-clearMarker` at CLI validation time or guard this branch to avoid modifying parameters when `isDryRun` is true.
##########
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/tools/metatool/TestHiveMetaToolCommandLine.java:
##########
@@ -154,6 +154,9 @@ public void testParseDedupColumns() throws ParseException {
cl = new HiveMetaToolCommandLine(new String[] {"-dedupColumns", "-timeout", "3600"});
assertEquals(Long.valueOf(3600L), cl.getDedupColumnsTimeoutSeconds());
+
+ cl = new HiveMetaToolCommandLine(new String[] {"-dedupColumns", "hive", "default", "person", "-clearMarker"});
+ assertTrue(cl.isClearMarker());
Review Comment:
There’s coverage for `-clearMarker` when `-dedupColumns` has all 3 optional args, but not for the common case where no catalog/db/table filters are provided (e.g., `-dedupColumns -clearMarker`). With Commons CLI optional-arg options, a following `-clearMarker` token can be accidentally parsed as an argument value in some configurations; adding a test for the no-filter case would ensure the new option is robust and parsed as intended.
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/tools/metatool/MetaToolTaskDedupColumns.java:
##########
@@ -69,10 +70,10 @@ void execute() {
daemon.setDaemon(true);
daemon.start();
}
- result = getObjectStore().dedupColumns(catalogFilter, dbFilter, tableFilter, progress, isDryRun, isVerbose);
- } catch (MetaException ex) {
- // Deadline.startTimer throws MetaException, this shouldn't happen as the Deadline has been registered.
- System.err.println(ex.getMessage());
+ result = getObjectStore().dedupColumns(catalogFilter, dbFilter, tableFilter, progress, isDryRun, isVerbose,
+ getCl().isClearMarker());
+ } catch (Throwable ex) {
+ ex.printStackTrace(System.err);
Review Comment:
Catching `Throwable` here will also swallow `Error`s and (more importantly) will prevent failures from propagating to `HiveMetaTool.main`, which can lead to an apparent “successful” run (exit code 0) after printing a stack trace. Consider catching `Exception` (not `Throwable`) and rethrowing after printing (or wrapping into a runtime exception), so the process terminates with non-zero status consistently on failures.
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/tools/ColumnDeduplicator.java:
##########
@@ -119,6 +132,39 @@ MetaToolObjectStore.DedupColumnsResult run(String catalogFilter, String dbFilter
return result;
}
+ public TableInfo getTable(long tableId) {
Review Comment:
Introducing a new `public` method on `ColumnDeduplicator` expands the production API surface for what appears to be an internal helper (it’s primarily used by tests in this PR). If external callers aren’t intended, consider reducing visibility (and relocating tests to the same package), or clearly marking it as internal/private API (e.g., via an audience annotation used in the project) to avoid accidental dependencies.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-30194: HiveMetaTool drops silently without clear error message [hive]
Posted by "dengzhhu653 (via GitHub)" <gi...@apache.org>.
dengzhhu653 commented on code in PR #6887:
URL: https://github.com/apache/hive/pull/6887#discussion_r4331837221
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/tools/ColumnDeduplicator.java:
##########
@@ -119,6 +132,32 @@ MetaToolObjectStore.DedupColumnsResult run(String catalogFilter, String dbFilter
return result;
}
+ private TableInfo getTable(long tableId) {
+ MTable table = pm.getObjectById(MTable.class, new MTable.PK(tableId));
+ if (table == null) {
+ return null;
+ }
+ Map<String, String> params = table.getParameters() == null ?
+ new HashMap<>() : table.getParameters();
+ // This table has been processed early
+ if (params.containsKey(COLUMN_DEDUPLICATOR_MARKER)) {
+ if (clearMarker) {
+ params.remove(COLUMN_DEDUPLICATOR_MARKER);
+ table.setParameters(params);
+ }
+ return null;
+ }
+
+ if (!isDryRun) {
+ params.put(COLUMN_DEDUPLICATOR_MARKER, "" + System.currentTimeMillis() / 1000);
+ table.setParameters(params);
+ }
Review Comment:
Good catch!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-30194: HiveMetaTool drops silently without clear error message [hive]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #6887:
URL: https://github.com/apache/hive/pull/6887#issuecomment-6213717835
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=6887) **Quality Gate passed**
Issues
 [15 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=6887&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=6887&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=6887&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=6887&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=6887&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=6887)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-30194: HiveMetaTool drops silently without clear error message [hive]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #6887:
URL: https://github.com/apache/hive/pull/6887#issuecomment-6308263979
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=6887) **Quality Gate passed**
Issues
 [11 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=6887&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=6887&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=6887&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=6887&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=6887&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=6887)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-30194: HiveMetaTool drops silently without clear error message [hive]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #6887:
URL: https://github.com/apache/hive/pull/6887#discussion_r4327527180
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/tools/ColumnDeduplicator.java:
##########
@@ -119,6 +132,32 @@ MetaToolObjectStore.DedupColumnsResult run(String catalogFilter, String dbFilter
return result;
}
+ private TableInfo getTable(long tableId) {
+ MTable table = pm.getObjectById(MTable.class, new MTable.PK(tableId));
+ if (table == null) {
+ return null;
+ }
+ Map<String, String> params = table.getParameters() == null ?
+ new HashMap<>() : table.getParameters();
+ // This table has been processed early
+ if (params.containsKey(COLUMN_DEDUPLICATOR_MARKER)) {
+ if (clearMarker) {
+ params.remove(COLUMN_DEDUPLICATOR_MARKER);
+ table.setParameters(params);
+ }
+ return null;
+ }
+
+ if (!isDryRun) {
+ params.put(COLUMN_DEDUPLICATOR_MARKER, "" + System.currentTimeMillis() / 1000);
+ table.setParameters(params);
+ }
Review Comment:
`-clearMarker` mode should not add the marker to tables that don’t already have it. With the current logic, if `clearMarker` is true and the table is unmarked, the marker will be added (unless `isDryRun`), which is the opposite of a “clear-only” operation. Consider short-circuiting early (e.g., return null) when `clearMarker` is true and the marker is absent, so the cleanup stage doesn’t mutate previously-unmarked tables.
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/tools/ColumnDeduplicator.java:
##########
@@ -229,25 +268,16 @@ private List<TableInfo> findPartitionedTables(String catalogFilter, String dbFil
Query query = !filter.isEmpty() ?
pm.newQuery(MPartition.class, filter.toString()) :
pm.newQuery(MPartition.class);
- query.setResult("DISTINCT this.table");
+ query.setResult("DISTINCT this.table.id");
boolean success = false;
- List<TableInfo> tables = new ArrayList<>();
+ List<Long> tables = new ArrayList<>();
store.openTransaction();
try {
- List<MTable> mTables = (List<MTable>) query.executeWithArray(parameterVals.toArray(new String[0]));
- pm.retrieveAll(mTables);
- for (MTable mTable : mTables) {
- pm.retrieve(mTable.getDatabase());
- pm.retrieve(mTable.getSd());
- pm.retrieve(mTable.getSd().getCD());
- tables.add(new TableInfo(
- mTable.getId(),
- mTable.getSd().getCD().getId(),
- mTable.getDatabase().getCatalogName(),
- mTable.getDatabase().getName(),
- mTable.getTableName()));
- }
+ tables.addAll((List<Long>) query.executeWithArray(parameterVals.toArray(new String[0])));
success = store.commitTransaction();
+ } catch (Throwable e) {
+ e.printStackTrace(System.err);
+ throw new RuntimeException(e);
Review Comment:
Throwing a `RuntimeException` here can terminate the metatool run unexpectedly (and bypass the `DedupColumnsResult` failure-reporting path that the rest of `run()` uses). To preserve the PR’s stated goal of being more error-tolerant, prefer propagating a checked exception type already handled by the caller (e.g., `MetaException`), or handle it inside `run()` by setting `result.catchException(e)` and returning a result with the captured throwable instead of crashing the process.
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/tools/metatool/HiveMetaTool.java:
##########
@@ -81,6 +81,7 @@ public static void main(String[] args) {
execute(args);
} catch (Exception e) {
status = -1;
+ e.printStackTrace(System.err);
LOGGER.error("Exception occured", e);
Review Comment:
This prints the full stack trace twice (once to `stderr`, once via the logger). If the intention is to ensure visibility in console output, consider printing a concise message to `stderr` (or guarding stacktrace printing behind a verbosity flag) while keeping the full stack trace in logs, to reduce noisy/duplicated output in operational environments.
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/tools/metatool/MetaToolTaskDedupColumns.java:
##########
@@ -69,7 +69,8 @@ void execute() {
daemon.setDaemon(true);
daemon.start();
}
- result = getObjectStore().dedupColumns(catalogFilter, dbFilter, tableFilter, progress, isDryRun, isVerbose);
+ result = getObjectStore().dedupColumns(catalogFilter, dbFilter, tableFilter, progress, isDryRun, isVerbose,
+ getCl().isClearMarker());
Review Comment:
Given `ColumnDeduplicator.findPartitionedTables()` now throws `RuntimeException` on failures, this call path can escape the existing `catch (MetaException ex)` below and abort the task without the intended structured error handling/summary. Either avoid throwing `RuntimeException` from the dedup implementation (preferred), or broaden the catch here to handle unchecked exceptions consistently so the command doesn’t fail in an uncontrolled way.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-30194: HiveMetaTool drops silently without clear error message [hive]
Posted by "dengzhhu653 (via GitHub)" <gi...@apache.org>.
dengzhhu653 commented on code in PR #6887:
URL: https://github.com/apache/hive/pull/6887#discussion_r4332976830
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/tools/metatool/MetaToolTaskDedupColumns.java:
##########
@@ -69,10 +70,10 @@ void execute() {
daemon.setDaemon(true);
daemon.start();
}
- result = getObjectStore().dedupColumns(catalogFilter, dbFilter, tableFilter, progress, isDryRun, isVerbose);
- } catch (MetaException ex) {
- // Deadline.startTimer throws MetaException, this shouldn't happen as the Deadline has been registered.
- System.err.println(ex.getMessage());
+ result = getObjectStore().dedupColumns(catalogFilter, dbFilter, tableFilter, progress, isDryRun, isVerbose,
+ getCl().isClearMarker());
+ } catch (Throwable ex) {
+ ex.printStackTrace(System.err);
Review Comment:
Without rethrowing the exception helps the task print the task summary at the end, and the exception will be propagated to HiveMetaTool in case of task failure
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-30194: HiveMetaTool drops silently without clear error message [hive]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #6887:
URL: https://github.com/apache/hive/pull/6887#issuecomment-6258721299
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=6887) **Quality Gate passed**
Issues
 [10 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=6887&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=6887&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=6887&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=6887&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=6887&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=6887)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-30194: HiveMetaTool drops silently without clear error message [hive]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #6887:
URL: https://github.com/apache/hive/pull/6887#discussion_r4331915091
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/tools/ColumnDeduplicator.java:
##########
@@ -119,6 +132,32 @@ MetaToolObjectStore.DedupColumnsResult run(String catalogFilter, String dbFilter
return result;
}
+ private TableInfo getTable(long tableId) {
+ MTable table = pm.getObjectById(MTable.class, new MTable.PK(tableId));
+ if (table == null) {
+ return null;
+ }
+ Map<String, String> params = table.getParameters() == null ?
+ new HashMap<>() : table.getParameters();
+ // This table has been processed early
+ if (params.containsKey(COLUMN_DEDUPLICATOR_MARKER)) {
+ if (clearMarker) {
+ params.remove(COLUMN_DEDUPLICATOR_MARKER);
+ table.setParameters(params);
+ }
+ return null;
+ }
+
+ if (!isDryRun) {
+ params.put(COLUMN_DEDUPLICATOR_MARKER, "" + System.currentTimeMillis() / 1000);
+ table.setParameters(params);
+ }
Review Comment:
`-clearMarker` is described as clearing an existing marker, but when `clearMarker == true` and a table *doesn’t* already have the marker, this code will still add the marker (and then proceed to deduplicate). That can unintentionally mark previously-unprocessed tables as processed, causing future runs to skip them. Fix by making `clearMarker` a true “cleanup-only” mode: if `clearMarker` is set, do not add markers (and ideally do not deduplicate); only remove the marker when present and otherwise no-op for that table.
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/tools/ColumnDeduplicator.java:
##########
@@ -56,43 +57,55 @@
* the same CD, reducing metadata bloat that can accumulate during replication.
*/
public final class ColumnDeduplicator {
+ public static final String COLUMN_DEDUPLICATOR_MARKER = "__metatool_column_deduplication__";
private final RawStore store;
private final PersistenceManager pm;
private final AtomicReference<String> progress;
private final boolean isDryRun;
private final boolean isVerbose;
+ private final boolean clearMarker;
public ColumnDeduplicator(RawStoreBundle bundle, AtomicReference<String> progress,
- boolean isDryRun, boolean isVerbose) {
+ boolean isDryRun, boolean isVerbose, boolean clearMarker) {
this.store = bundle.getBaseStore();
this.pm = bundle.getPersistentManager();
this.progress = progress;
this.isDryRun = isDryRun;
this.isVerbose = isVerbose;
+ this.clearMarker = clearMarker;
}
MetaToolObjectStore.DedupColumnsResult run(String catalogFilter, String dbFilter, String tableFilter) {
- List<TableInfo> tables = findPartitionedTables(catalogFilter, dbFilter, tableFilter);
+ List<Long> tables = findPartitionedTables(catalogFilter, dbFilter, tableFilter);
MetaToolObjectStore.DedupColumnsResult result = new MetaToolObjectStore.DedupColumnsResult(tables.size());
long start = System.currentTimeMillis();
for (int i = 0; i < tables.size() && result.getException() == null; i++) {
boolean committed = false;
- TableInfo table = tables.get(i);
+ long tableId = tables.get(i);
PostCommitValidation validation = null;
+ TableInfo table = null;
store.openTransaction();
try {
Deadline.checkTimeout();
- validation = deduplicateTable(table, result);
+ table = getTable(tableId);
+ // This target table might be dropped or deduplicated, ignore
+ // If this is the stage to clean up the marker, ignore
+ if (table != null) {
+ validation = deduplicateTable(table, result);
+ }
committed = store.commitTransaction();
- } catch (Exception ex) {
+ } catch (Throwable ex) {
+ System.err.println("Failed to deduplicate the column in this table: " + (table == null ? tableId : table));
+ ex.printStackTrace(System.err);
result.catchException(ex);
Review Comment:
Catching `Throwable` will also catch serious `Error`s (e.g., `OutOfMemoryError`, `StackOverflowError`) that typically should not be swallowed and converted into a normal tool failure. Prefer catching `Exception` (or a narrower set) here; if you must catch `Throwable` for logging, rethrow when `ex instanceof Error` after logging so the process can fail fast in unrecoverable situations.
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/tools/MetaToolObjectStore.java:
##########
@@ -977,7 +977,7 @@ public static final class DedupColumnsResult {
private int columnDescriptorsRemoved;
private int tablesValidated;
private final List<String> details = new ArrayList<>();
- private Exception exception;
+ private Throwable exception;
Review Comment:
Changing `DedupColumnsResult` to store/return `Throwable` broadens the contract to include `Error`s, which generally aren’t meant to be treated as reportable/handled outcomes. Consider keeping the result API typed to `Exception` (or a project-specific checked/unchecked exception type) and handle `Error`s separately (log + rethrow) to avoid encouraging callers to treat JVM-fatal conditions as routine results.
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/tools/MetaToolObjectStore.java:
##########
@@ -1025,18 +1025,18 @@ void addDetail(String detail) {
details.add(detail);
}
- void catchException(Exception exception) {
+ void catchException(Throwable exception) {
this.exception = exception;
}
- public Exception getException() {
+ public Throwable getException() {
return exception;
}
Review Comment:
Changing `DedupColumnsResult` to store/return `Throwable` broadens the contract to include `Error`s, which generally aren’t meant to be treated as reportable/handled outcomes. Consider keeping the result API typed to `Exception` (or a project-specific checked/unchecked exception type) and handle `Error`s separately (log + rethrow) to avoid encouraging callers to treat JVM-fatal conditions as routine results.
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/tools/ColumnDeduplicator.java:
##########
@@ -119,6 +132,32 @@ MetaToolObjectStore.DedupColumnsResult run(String catalogFilter, String dbFilter
return result;
}
+ private TableInfo getTable(long tableId) {
+ MTable table = pm.getObjectById(MTable.class, new MTable.PK(tableId));
+ if (table == null) {
+ return null;
+ }
Review Comment:
`PersistenceManager.getObjectById(...)` commonly throws (e.g., `JDOObjectNotFoundException`) when the row doesn’t exist rather than returning `null`. Since the intent is to ignore dropped tables, this should explicitly handle the not-found case (catch the specific exception and return null, or use an API variant that doesn’t throw) to avoid aborting the whole run when a table disappears mid-scan.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org