amansinha100 commented on code in PR #6577:
URL: https://github.com/apache/hive/pull/6577#discussion_r3540681189
##########
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java:
##########
@@ -312,12 +311,12 @@ public enum ConfVars {
ACID_HOUSEKEEPER_SERVICE_INTERVAL("metastore.acid.housekeeper.interval",
"hive.metastore.acid.housekeeper.interval", 60, TimeUnit.SECONDS,
"Time interval describing how often the acid housekeeper runs."),
-
COMPACTION_HOUSEKEEPER_SERVICE_INTERVAL("metastore.compaction.housekeeper.interval",
- "hive.metastore.compaction.housekeeper.interval", 300,
TimeUnit.SECONDS,
- "Time interval describing how often the acid compaction housekeeper
runs."),
ACID_TXN_CLEANER_INTERVAL("metastore.acid.txn.cleaner.interval",
"hive.metastore.acid.txn.cleaner.interval", 10, TimeUnit.SECONDS,
"Time interval describing how often aborted and committed txns are
cleaned."),
+
ADD_PARTITION_REUSE_EXISTING_COLUMN_DESCRIPTORS("metastore.add.partition.reuse.existing.column.descriptors",
Review Comment:
A few thoughts on simplifying the config name (it seems verbose) and the
functionality, default value:
- the word 'existing' is already implied by 'reuse' since one can only reuse
existing descriptors, so you could remove 'existing'.
- would this functionality only apply for explicit ADD PARTITION commands
or would it also apply for MSCK REPAIR which also adds partitions under the
covers that are missing in the metastore but are present in the filesystem.
Once MSCK REPAIR adds partitions, then subsequent schema evolutions of those
partitions should ideally also be reusing the column descriptors. But if this
is not currently supported or tested, it is ok to file a separate ticket for
followup.
- from an end user perspective, ADD COLUMNS and CHANGE COLUMN are also
actions that trigger this duplicate column descriptors, so it may not be
accurate to say 'add.partition' . Considering all these, we could simplify it
to `metastore.partition.reuse.column.descriptors`. WDYT ?
- About the default value, since this is changing the generated SQL, it
would be better to give it some more time for testing - not just functionally
but also performance with order of 100K partitions. Metastore sql changes have
significant impact and is in some sense a behavior change, so would be good to
wait for further testing.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
