You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@hive.apache.org by "DanielZhu58 (via GitHub)" <gi...@apache.org> on 2025/07/09 23:18:39 UTC
[PR] HIVE-28813: MSCK/Analyze commands can show a warning in console for Small files. [hive]
DanielZhu58 opened a new pull request, #5950:
URL: https://github.com/apache/hive/pull/5950
### What changes were proposed in this pull request?
Add warning when MSCK/Analyze commands can show a warning in console for table/partition if (totalSize / numFiles) is less than avgFileSize.
### Why are the changes needed?
So the users can know if there are small files.
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
Manual tests.
--
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-28813: MSCK/Analyze commands can show a warning in console for Small files. [hive]
Posted by "DanielZhu58 (via GitHub)" <gi...@apache.org>.
DanielZhu58 commented on code in PR #5950:
URL: https://github.com/apache/hive/pull/5950#discussion_r2443574514
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/Msck.java:
##########
@@ -240,6 +246,76 @@ public int repair(MsckInfo msckInfo) throws TException, MetastoreException, IOEx
}
}
+ // Generate small files warnings only for partsNotInMs
+ try {
+ // Threshold in bytes for average file size considered "small"
+ final long threshold =
+ MetastoreConf.getLongVar(getConf(), MetastoreConf.ConfVars.MSCK_SMALLFILES_AVG_SIZE);
+
+ // Collect partition names
+ final List<String> names = new ArrayList<>(partsNotInMs.size());
+ for (CheckResult.PartitionResult pr : partsNotInMs) {
+ final String name = pr.getPartitionName();
+ if (name != null && !name.isEmpty()) {
+ names.add(name);
+ }
+ }
+
+ // Batch over the names to limit single-RPC payload.
+ final int BATCH = 1000;
+ final Map<String, Partition> byName = new HashMap<>(names.size() * 2);
+
+ for (int i = 0; i < names.size(); i += BATCH) {
+ final List<String> batch = names.subList(i, Math.min(i + BATCH, names.size()));
+ final GetPartitionsByNamesRequest req = new GetPartitionsByNamesRequest(table.getDbName(), table.getTableName());
+ req.setNames(batch);
+
+ // In this branch, getPartitionsByNames returns List<Partition>
+ List<Partition> plist;
+ try {
+ @SuppressWarnings("unchecked")
+ List<Partition> tmp = (List<Partition>) getMsc().getPartitionsByNames(req);
+ plist = (tmp != null) ? tmp : Collections.emptyList();
+ } catch (NoSuchObjectException e) {
+ plist = Collections.emptyList();
+ }
+
+ for (Partition p : plist) {
+ final String pName = Warehouse.makePartName(table.getPartitionKeys(), p.getValues());
+ byName.put(pName, p);
+ }
+ }
+
+ // Build small-files stats for partitions that have quick HMS stats.
+ final Map<String, String> smallFilesStats = new TreeMap<>();
+
+ for (String pName : names) {
Review Comment:
They are pretty much the same.
We need to do `smallFilesStats.put(pName, "avgBytes=" + avg + ", partition total files=" + numFiles + ", totalBytes=" + totalSize);` later, so it's easier to use pName to iterate.
--
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-28813: MSCK/Analyze commands can show a warning in console for Small files. [hive]
Posted by "dengzhhu653 (via GitHub)" <gi...@apache.org>.
dengzhhu653 commented on code in PR #5950:
URL: https://github.com/apache/hive/pull/5950#discussion_r2370992296
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/Msck.java:
##########
@@ -239,6 +244,57 @@ public int repair(MsckInfo msckInfo) throws TException, MetastoreException, IOEx
throw new MetastoreException(e);
}
}
+ // Generate small files warnings only for partsNotInMs
+ try {
+ long threshold = HiveConf.getLongVar(getConf(), HiveConf.ConfVars.HIVE_MERGE_MAP_FILES_AVG_SIZE);
+ Map<String, String> smallFilesStats = new TreeMap<>();
+
+ for (CheckResult.PartitionResult pr : partsNotInMs) {
+ List<String> statsVals;
+ try {
+ // parse the partition name into a few partition values
+ statsVals = Warehouse.makeValsFromName(pr.getPartitionName(), null);
+ } catch (MetaException me) {
+ // skip this partition if this somehow failed
+ continue;
+ }
+
+ Partition p;
+ try {
+ // the newly added partition should exist in metastore
+ p = getMsc().getPartition(table.getCatName(), table.getDbName(), table.getTableName(), statsVals);
+ } catch (NoSuchObjectException nsoe) {
+ // the partition is not actually added in metastore
+ continue;
+ }
+
+ Path loc = new Path(p.getSd().getLocation());
Review Comment:
we have the partition location in `pr`
--
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-28813: MSCK/Analyze commands can show a warning in console for Small files. [hive]
Posted by "dengzhhu653 (via GitHub)" <gi...@apache.org>.
dengzhhu653 commented on code in PR #5950:
URL: https://github.com/apache/hive/pull/5950#discussion_r2370983642
##########
standalone-metastore/metastore-server/pom.xml:
##########
@@ -470,6 +470,12 @@
<artifactId>mssqlserver</artifactId>
<scope>test</scope>
</dependency>
+ <dependency>
+ <groupId>org.apache.hive</groupId>
Review Comment:
why need this dependency?
--
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-28813: MSCK/Analyze commands can show a warning in console for Small files. [hive]
Posted by "DanielZhu58 (via GitHub)" <gi...@apache.org>.
DanielZhu58 commented on code in PR #5950:
URL: https://github.com/apache/hive/pull/5950#discussion_r2383194801
##########
ql/src/java/org/apache/hadoop/hive/ql/stats/BasicStatsNoJobTask.java:
##########
@@ -181,6 +184,35 @@ public void run() {
}
parameters.putAll(basicStatistics);
StatsSetupConst.setBasicStatsState(parameters, StatsSetupConst.TRUE);
+ try {
+ String ts = parameters.get(StatsSetupConst.TOTAL_SIZE);
Review Comment:
Let me think about it.
If I decide to refactor it, I will upload it in my next commit.
--
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-28813: MSCK/Analyze commands can show a warning in console for Small files. [hive]
Posted by "difin (via GitHub)" <gi...@apache.org>.
difin commented on code in PR #5950:
URL: https://github.com/apache/hive/pull/5950#discussion_r2311713304
##########
ql/src/java/org/apache/hadoop/hive/ql/ddl/misc/msck/MsckOperation.java:
##########
@@ -75,7 +78,25 @@ public int execute() throws HiveException, IOException, TException, MetastoreExc
MsckInfo msckInfo = new MsckInfo(SessionState.get().getCurrentCatalog(), tableName.getDb(), tableName.getTable(),
desc.getFilterExp(), desc.getResFile(), desc.isRepairPartitions(),
desc.isAddPartitions(), desc.isDropPartitions(), partitionExpirySeconds);
- return msck.repair(msckInfo);
+ int result = msck.repair(msckInfo);
+ if (msckInfo.getSmallFilesStats() != null && !msckInfo.getSmallFilesStats().isEmpty()){
+ List<String> logInfo = new ArrayList<>(msckInfo.getSmallFilesStats().size());
+ for (Map.Entry<String, String> me : msckInfo.getSmallFilesStats().entrySet()){
+ String partitionName = me.getKey();
+ String smallFilesStats = me.getValue();
+ logInfo.add("This table/partition average file size is less than Hive average file size.\n " +
+ "The partition name is " + partitionName + ". " + smallFilesStats);
+ }
+ SessionState ss = SessionState.get();
+ if (ss != null && ss.getConsole() != null) {
+ ss.getConsole().printInfo("[MSCK] Small files detected.");
+ for (String line : logInfo) {
+ ss.getConsole().printInfo("[MSCK] " + line);
+ }
Review Comment:
Should it be in functional style `logInfo.stream().forEach(...);`?
--
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-28813: MSCK/Analyze commands can show a warning in console for Small files. [hive]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #5950:
URL: https://github.com/apache/hive/pull/5950#issuecomment-3270678777
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5950) **Quality Gate passed**
Issues
 [10 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5950&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5950&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=5950&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5950&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5950&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5950)
--
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-28813: MSCK/Analyze commands can show a warning in console for Small files. [hive]
Posted by "dengzhhu653 (via GitHub)" <gi...@apache.org>.
dengzhhu653 commented on code in PR #5950:
URL: https://github.com/apache/hive/pull/5950#discussion_r2430881136
##########
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java:
##########
@@ -1821,6 +1821,10 @@ public enum ConfVars {
"A ZooKeeper instance must be up and running when using zookeeper Hive lock manager "),
HIVE_TXN_STATS_ENABLED("hive.txn.stats.enabled", "hive.txn.stats.enabled", true,
"Whether Hive supports transactional stats (accurate stats for transactional tables)"),
+ MSCK_SMALLFILES_AVG_SIZE("metastore.msck.smallfiles.avgsize", "metastore.msck.smallfiles.avgsize", (long) (16 * 1000 * 1000),
Review Comment:
if we are not able to access HiveConf HIVE_MERGE_MAP_FILES_AVG_SIZE in Metastore, we can use `conf.get("hive.merge.smallfiles.avgsize", "16000000")` compared a new property
--
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-28813: MSCK/Analyze commands can show a warning in console for Small files. [hive]
Posted by "DanielZhu58 (via GitHub)" <gi...@apache.org>.
DanielZhu58 commented on code in PR #5950:
URL: https://github.com/apache/hive/pull/5950#discussion_r2278640093
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/MsckInfo.java:
##########
@@ -33,6 +36,8 @@ public class MsckInfo {
private final boolean dropPartitions;
private final long partitionExpirySeconds;
+ private Map<String, String> smallFilesStats = new HashMap<>();
Review Comment:
We need to show the end user that in this specific partition, there is a total size, and there is a total number of files, which means there are many small files existing in this partition.
And that can help the end user when they do the "msck" operation.
--
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-28813: MSCK/Analyze commands can show a warning in console for Small files. [hive]
Posted by "difin (via GitHub)" <gi...@apache.org>.
difin commented on code in PR #5950:
URL: https://github.com/apache/hive/pull/5950#discussion_r2311710381
##########
ql/src/java/org/apache/hadoop/hive/ql/ddl/misc/msck/MsckOperation.java:
##########
@@ -75,7 +78,25 @@ public int execute() throws HiveException, IOException, TException, MetastoreExc
MsckInfo msckInfo = new MsckInfo(SessionState.get().getCurrentCatalog(), tableName.getDb(), tableName.getTable(),
desc.getFilterExp(), desc.getResFile(), desc.isRepairPartitions(),
desc.isAddPartitions(), desc.isDropPartitions(), partitionExpirySeconds);
- return msck.repair(msckInfo);
+ int result = msck.repair(msckInfo);
+ if (msckInfo.getSmallFilesStats() != null && !msckInfo.getSmallFilesStats().isEmpty()){
+ List<String> logInfo = new ArrayList<>(msckInfo.getSmallFilesStats().size());
+ for (Map.Entry<String, String> me : msckInfo.getSmallFilesStats().entrySet()){
+ String partitionName = me.getKey();
+ String smallFilesStats = me.getValue();
+ logInfo.add("This table/partition average file size is less than Hive average file size.\n " +
+ "The partition name is " + partitionName + ". " + smallFilesStats);
+ }
Review Comment:
It would be more modern to re-implement it in a functional style:
`List<String> logInfo = msckInfo.getSmallFilesStats().stream().forEach(...).toList();`
--
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-28813: MSCK/Analyze commands can show a warning in console for Small files. [hive]
Posted by "dengzhhu653 (via GitHub)" <gi...@apache.org>.
dengzhhu653 commented on code in PR #5950:
URL: https://github.com/apache/hive/pull/5950#discussion_r2371017645
##########
ql/src/java/org/apache/hadoop/hive/ql/stats/BasicStatsNoJobTask.java:
##########
@@ -181,6 +184,35 @@ public void run() {
}
parameters.putAll(basicStatistics);
StatsSetupConst.setBasicStatsState(parameters, StatsSetupConst.TRUE);
+ try {
+ String ts = parameters.get(StatsSetupConst.TOTAL_SIZE);
Review Comment:
the code is almost the same as `BasicStatsTask`, can we be able to make it an util method
##########
ql/src/java/org/apache/hadoop/hive/ql/stats/BasicStatsNoJobTask.java:
##########
@@ -181,6 +184,35 @@ public void run() {
}
parameters.putAll(basicStatistics);
StatsSetupConst.setBasicStatsState(parameters, StatsSetupConst.TRUE);
+ try {
+ String ts = parameters.get(StatsSetupConst.TOTAL_SIZE);
Review Comment:
the code is almost the same as `BasicStatsTask`, can we able to make it an util method
--
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-28813: MSCK/Analyze commands can show a warning in console for Small files. [hive]
Posted by "DanielZhu58 (via GitHub)" <gi...@apache.org>.
DanielZhu58 commented on code in PR #5950:
URL: https://github.com/apache/hive/pull/5950#discussion_r2332730888
##########
ql/src/java/org/apache/hadoop/hive/ql/ddl/misc/msck/MsckOperation.java:
##########
@@ -75,7 +78,25 @@ public int execute() throws HiveException, IOException, TException, MetastoreExc
MsckInfo msckInfo = new MsckInfo(SessionState.get().getCurrentCatalog(), tableName.getDb(), tableName.getTable(),
desc.getFilterExp(), desc.getResFile(), desc.isRepairPartitions(),
desc.isAddPartitions(), desc.isDropPartitions(), partitionExpirySeconds);
- return msck.repair(msckInfo);
+ int result = msck.repair(msckInfo);
+ if (msckInfo.getSmallFilesStats() != null && !msckInfo.getSmallFilesStats().isEmpty()){
+ List<String> logInfo = new ArrayList<>(msckInfo.getSmallFilesStats().size());
+ for (Map.Entry<String, String> me : msckInfo.getSmallFilesStats().entrySet()){
+ String partitionName = me.getKey();
+ String smallFilesStats = me.getValue();
+ logInfo.add("This table/partition average file size is less than Hive average file size.\n " +
+ "The partition name is " + partitionName + ". " + smallFilesStats);
+ }
+ SessionState ss = SessionState.get();
+ if (ss != null && ss.getConsole() != null) {
+ ss.getConsole().printInfo("[MSCK] Small files detected.");
+ for (String line : logInfo) {
+ ss.getConsole().printInfo("[MSCK] " + line);
+ }
Review Comment:
Acknowledged. I will use` logInfo.forEach(line -> ss.getConsole().printInfo("[MSCK] " + line));`.
--
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-28813: MSCK/Analyze commands can show a warning in console for Small files. [hive]
Posted by "DanielZhu58 (via GitHub)" <gi...@apache.org>.
DanielZhu58 commented on code in PR #5950:
URL: https://github.com/apache/hive/pull/5950#discussion_r2332732581
##########
ql/src/java/org/apache/hadoop/hive/ql/ddl/misc/msck/MsckOperation.java:
##########
@@ -75,7 +78,25 @@ public int execute() throws HiveException, IOException, TException, MetastoreExc
MsckInfo msckInfo = new MsckInfo(SessionState.get().getCurrentCatalog(), tableName.getDb(), tableName.getTable(),
desc.getFilterExp(), desc.getResFile(), desc.isRepairPartitions(),
desc.isAddPartitions(), desc.isDropPartitions(), partitionExpirySeconds);
- return msck.repair(msckInfo);
+ int result = msck.repair(msckInfo);
+ if (msckInfo.getSmallFilesStats() != null && !msckInfo.getSmallFilesStats().isEmpty()){
+ List<String> logInfo = new ArrayList<>(msckInfo.getSmallFilesStats().size());
+ for (Map.Entry<String, String> me : msckInfo.getSmallFilesStats().entrySet()){
+ String partitionName = me.getKey();
+ String smallFilesStats = me.getValue();
+ logInfo.add("This table/partition average file size is less than Hive average file size.\n " +
+ "The partition name is " + partitionName + ". " + smallFilesStats);
+ }
Review Comment:
Acknowledged.
--
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-28813: MSCK/Analyze commands can show a warning in console for Small files. [hive]
Posted by "dengzhhu653 (via GitHub)" <gi...@apache.org>.
dengzhhu653 commented on code in PR #5950:
URL: https://github.com/apache/hive/pull/5950#discussion_r2444073519
##########
ql/src/java/org/apache/hadoop/hive/ql/stats/BasicStatsTask.java:
##########
@@ -182,6 +187,27 @@ public Object process(StatsAggregator statsAggregator) throws HiveException, Met
parameters.putAll(providedBasicStats);
}
+ try {
+ final long threshold = (conf != null)
+ ? conf.getLongVar(HiveConf.ConfVars.HIVE_MERGE_MAP_FILES_AVG_SIZE)
+ : HiveConf.ConfVars.HIVE_MERGE_MAP_FILES_AVG_SIZE.defaultLongVal;
+
+ final String who = (p.getPartition() == null)
+ ? "table " + p.getTable().getFullyQualifiedName()
+ : "partition " + p.getPartition().getName();
+
+ SmallFilesWarningUtil.smallFilesWarnings(parameters, 100L, threshold, who, "[ANALYZE]")
+ .ifPresent(msg -> {
+ LOG.warn(msg);
+ SessionState ss = SessionState.get();
+ if (ss != null && ss.getConsole() != null) {
+ ss.getConsole().printInfo(msg);
+ }
+ });
+ } catch (Exception e) {
Review Comment:
nit: remove this `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-28813: MSCK/Analyze commands can show a warning in console for Small files. [hive]
Posted by "dengzhhu653 (via GitHub)" <gi...@apache.org>.
dengzhhu653 commented on code in PR #5950:
URL: https://github.com/apache/hive/pull/5950#discussion_r2443656282
##########
ql/src/java/org/apache/hadoop/hive/ql/stats/BasicStatsNoJobTask.java:
##########
@@ -293,6 +307,15 @@ public void run() {
parameters.put(StatsSetupConst.NUM_FILES, String.valueOf(numFiles));
parameters.put(StatsSetupConst.NUM_ERASURE_CODED_FILES, String.valueOf(numErasureCodedFiles));
+ try {
+ String who = (partish.getPartition() == null) ? ("table " + partish.getTable().getFullyQualifiedName())
+ : ("partition " + partish.getPartition().getName());
+ long threshold = conf.getLongVar(HiveConf.ConfVars.HIVE_MERGE_MAP_FILES_AVG_SIZE);
+ SmallFilesWarningUtil.smallFilesWarnings(parameters, 100L, threshold, who, "[ANALYZE][NOSCAN]")
+ .ifPresent(smallFilesConsoleAndWarn(LOG));
+ } catch (Throwable t) {
Review Comment:
remove `catch (Throwable t)`?
--
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-28813: MSCK/Analyze commands can show a warning in console for Small files. [hive]
Posted by "DanielZhu58 (via GitHub)" <gi...@apache.org>.
DanielZhu58 commented on code in PR #5950:
URL: https://github.com/apache/hive/pull/5950#discussion_r2329526755
##########
ql/src/java/org/apache/hadoop/hive/ql/ddl/misc/msck/MsckOperation.java:
##########
@@ -75,7 +78,25 @@ public int execute() throws HiveException, IOException, TException, MetastoreExc
MsckInfo msckInfo = new MsckInfo(SessionState.get().getCurrentCatalog(), tableName.getDb(), tableName.getTable(),
desc.getFilterExp(), desc.getResFile(), desc.isRepairPartitions(),
desc.isAddPartitions(), desc.isDropPartitions(), partitionExpirySeconds);
- return msck.repair(msckInfo);
+ int result = msck.repair(msckInfo);
+ if (msckInfo.getSmallFilesStats() != null && !msckInfo.getSmallFilesStats().isEmpty()){
+ List<String> logInfo = new ArrayList<>(msckInfo.getSmallFilesStats().size());
+ for (Map.Entry<String, String> me : msckInfo.getSmallFilesStats().entrySet()){
+ String partitionName = me.getKey();
+ String smallFilesStats = me.getValue();
+ logInfo.add("This table/partition average file size is less than Hive average file size.\n " +
+ "The partition name is " + partitionName + ". " + smallFilesStats);
+ }
+ SessionState ss = SessionState.get();
+ if (ss != null && ss.getConsole() != null) {
+ ss.getConsole().printInfo("[MSCK] Small files detected.");
+ for (String line : logInfo) {
+ ss.getConsole().printInfo("[MSCK] " + line);
+ }
+ }
+ LOG.info("There are small files exist.\n{}", String.join("\n", logInfo));
Review Comment:
The text warnings are printed to the end user in the console.
So this doesn't have to be warnings in the LOG, and small files are common to see.
--
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-28813: MSCK/Analyze commands can show a warning in console for Small files. [hive]
Posted by "dengzhhu653 (via GitHub)" <gi...@apache.org>.
dengzhhu653 commented on code in PR #5950:
URL: https://github.com/apache/hive/pull/5950#discussion_r2437798189
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/Msck.java:
##########
@@ -240,6 +246,76 @@ public int repair(MsckInfo msckInfo) throws TException, MetastoreException, IOEx
}
}
+ // Generate small files warnings only for partsNotInMs
+ try {
+ // Threshold in bytes for average file size considered "small"
+ final long threshold =
+ MetastoreConf.getLongVar(getConf(), MetastoreConf.ConfVars.MSCK_SMALLFILES_AVG_SIZE);
+
+ // Collect partition names
+ final List<String> names = new ArrayList<>(partsNotInMs.size());
+ for (CheckResult.PartitionResult pr : partsNotInMs) {
+ final String name = pr.getPartitionName();
+ if (name != null && !name.isEmpty()) {
+ names.add(name);
+ }
+ }
+
+ // Batch over the names to limit single-RPC payload.
+ final int BATCH = 1000;
Review Comment:
why not use `Batchable`?
--
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-28813: MSCK/Analyze commands can show a warning in console for Small files. [hive]
Posted by "DanielZhu58 (via GitHub)" <gi...@apache.org>.
DanielZhu58 commented on code in PR #5950:
URL: https://github.com/apache/hive/pull/5950#discussion_r2443557938
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/Msck.java:
##########
@@ -240,6 +246,76 @@ public int repair(MsckInfo msckInfo) throws TException, MetastoreException, IOEx
}
}
+ // Generate small files warnings only for partsNotInMs
+ try {
+ // Threshold in bytes for average file size considered "small"
+ final long threshold =
+ MetastoreConf.getLongVar(getConf(), MetastoreConf.ConfVars.MSCK_SMALLFILES_AVG_SIZE);
+
+ // Collect partition names
+ final List<String> names = new ArrayList<>(partsNotInMs.size());
+ for (CheckResult.PartitionResult pr : partsNotInMs) {
+ final String name = pr.getPartitionName();
+ if (name != null && !name.isEmpty()) {
+ names.add(name);
+ }
+ }
+
+ // Batch over the names to limit single-RPC payload.
+ final int BATCH = 1000;
Review Comment:
Acknowledged. The Batchable way is added.
--
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-28813: MSCK/Analyze commands can show a warning in console for Small files. [hive]
Posted by "DanielZhu58 (via GitHub)" <gi...@apache.org>.
DanielZhu58 commented on code in PR #5950:
URL: https://github.com/apache/hive/pull/5950#discussion_r2443594335
##########
ql/src/java/org/apache/hadoop/hive/ql/stats/BasicStatsTask.java:
##########
@@ -182,6 +186,59 @@ public Object process(StatsAggregator statsAggregator) throws HiveException, Met
parameters.putAll(providedBasicStats);
}
+ try {
+ long totalSize = 0L, numFiles = 0L;
+ // Read stats from partition parameters only
+ // Parameters were already populated via populateQuickStats or providedBasicStats.putAll
+ String ts = parameters.get(StatsSetupConst.TOTAL_SIZE);
+ String nf = parameters.get(StatsSetupConst.NUM_FILES);
+
+ if (ts != null && nf != null) {
+ try {
+ totalSize = Long.parseLong(ts);
+ numFiles = Long.parseLong(nf);
+ } catch (NumberFormatException ignore) {
+ // If parsing fails, skip the warning without affecting the main flow
+ totalSize = 0L;
+ numFiles = 0L;
+ }
+ }
+
+ // Only warn when there are multiple files and total size > 0
+ if (numFiles > 100 && totalSize > 0) {
+ long threshold = (conf != null)
+ ? conf.getLongVar(HiveConf.ConfVars.HIVE_MERGE_MAP_FILES_AVG_SIZE)
+ : HiveConf.ConfVars.HIVE_MERGE_MAP_FILES_AVG_SIZE.defaultLongVal;
+
+ long avg = totalSize / numFiles;
+
+ if (avg <= threshold) {
Review Comment:
Acknowledged.
--
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-28813: MSCK/Analyze commands can show a warning in console for Small files. [hive]
Posted by "dengzhhu653 (via GitHub)" <gi...@apache.org>.
dengzhhu653 commented on code in PR #5950:
URL: https://github.com/apache/hive/pull/5950#discussion_r2443655717
##########
ql/src/java/org/apache/hadoop/hive/ql/stats/BasicStatsNoJobTask.java:
##########
@@ -181,6 +186,15 @@ public void run() {
}
parameters.putAll(basicStatistics);
StatsSetupConst.setBasicStatsState(parameters, StatsSetupConst.TRUE);
+ try {
+ String who = (partish.getPartition() == null) ? ("table " + partish.getTable().getFullyQualifiedName())
+ : ("partition " + partish.getPartition().getName());
+ long threshold = conf.getLongVar(HiveConf.ConfVars.HIVE_MERGE_MAP_FILES_AVG_SIZE);
+ SmallFilesWarningUtil.smallFilesWarnings(parameters, 100L, threshold, who, "[ANALYZE][NOSCAN]")
+ .ifPresent(smallFilesConsoleAndWarn(LOG));
+ } catch (Exception e) {
Review Comment:
if there is no exception assumed from the block, why not move the `try{ } 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-28813: MSCK/Analyze commands can show a warning in console for Small files. [hive]
Posted by "dengzhhu653 (via GitHub)" <gi...@apache.org>.
dengzhhu653 commented on code in PR #5950:
URL: https://github.com/apache/hive/pull/5950#discussion_r2444075850
##########
ql/src/java/org/apache/hadoop/hive/ql/stats/BasicStatsNoJobTask.java:
##########
@@ -30,6 +30,7 @@
import java.util.concurrent.ThreadFactory;
import java.util.concurrent.ThreadPoolExecutor;
import java.util.concurrent.TimeUnit;
+import java.util.function.Consumer;
Review Comment:
nit: import is unnecessary
--
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-28813: MSCK/Analyze commands can show a warning in console for Small files. [hive]
Posted by "DanielZhu58 (via GitHub)" <gi...@apache.org>.
DanielZhu58 commented on code in PR #5950:
URL: https://github.com/apache/hive/pull/5950#discussion_r2444097611
##########
ql/src/java/org/apache/hadoop/hive/ql/stats/BasicStatsNoJobTask.java:
##########
@@ -30,6 +30,7 @@
import java.util.concurrent.ThreadFactory;
import java.util.concurrent.ThreadPoolExecutor;
import java.util.concurrent.TimeUnit;
+import java.util.function.Consumer;
Review Comment:
Acknowledged.
--
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-28813: MSCK/Analyze commands can show a warning in console for Small files. [hive]
Posted by "DanielZhu58 (via GitHub)" <gi...@apache.org>.
DanielZhu58 commented on code in PR #5950:
URL: https://github.com/apache/hive/pull/5950#discussion_r2435021512
##########
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java:
##########
@@ -1821,6 +1821,10 @@ public enum ConfVars {
"A ZooKeeper instance must be up and running when using zookeeper Hive lock manager "),
HIVE_TXN_STATS_ENABLED("hive.txn.stats.enabled", "hive.txn.stats.enabled", true,
"Whether Hive supports transactional stats (accurate stats for transactional tables)"),
+ MSCK_SMALLFILES_AVG_SIZE("metastore.msck.smallfiles.avgsize", "metastore.msck.smallfiles.avgsize", (long) (16 * 1000 * 1000),
Review Comment:
This is part of the design of this PR. We can add this conf here, it can also be used in any future use.
--
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-28813: MSCK/Analyze commands can show a warning in console for Small files. [hive]
Posted by "dengzhhu653 (via GitHub)" <gi...@apache.org>.
dengzhhu653 commented on code in PR #5950:
URL: https://github.com/apache/hive/pull/5950#discussion_r2370998241
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/Msck.java:
##########
@@ -239,6 +244,57 @@ public int repair(MsckInfo msckInfo) throws TException, MetastoreException, IOEx
throw new MetastoreException(e);
}
}
+ // Generate small files warnings only for partsNotInMs
+ try {
+ long threshold = HiveConf.getLongVar(getConf(), HiveConf.ConfVars.HIVE_MERGE_MAP_FILES_AVG_SIZE);
+ Map<String, String> smallFilesStats = new TreeMap<>();
+
+ for (CheckResult.PartitionResult pr : partsNotInMs) {
+ List<String> statsVals;
+ try {
+ // parse the partition name into a few partition values
+ statsVals = Warehouse.makeValsFromName(pr.getPartitionName(), null);
+ } catch (MetaException me) {
+ // skip this partition if this somehow failed
+ continue;
+ }
+
+ Partition p;
+ try {
+ // the newly added partition should exist in metastore
+ p = getMsc().getPartition(table.getCatName(), table.getDbName(), table.getTableName(), statsVals);
+ } catch (NoSuchObjectException nsoe) {
+ // the partition is not actually added in metastore
+ continue;
+ }
+
+ Path loc = new Path(p.getSd().getLocation());
+ try {
+ FileSystem fs = loc.getFileSystem(getConf());
Review Comment:
we might need a new property to disable the stats gather if can
--
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-28813: MSCK/Analyze commands can show a warning in console for Small files. [hive]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #5950:
URL: https://github.com/apache/hive/pull/5950#issuecomment-3422156845
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5950) **Quality Gate passed**
Issues
 [61 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5950&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5950&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=5950&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5950&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5950&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5950)
--
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-28813: MSCK/Analyze commands can show a warning in console for Small files. [hive]
Posted by "DanielZhu58 (via GitHub)" <gi...@apache.org>.
DanielZhu58 commented on code in PR #5950:
URL: https://github.com/apache/hive/pull/5950#discussion_r2381250631
##########
ql/src/java/org/apache/hadoop/hive/ql/stats/BasicStatsTask.java:
##########
@@ -182,6 +184,48 @@ public Object process(StatsAggregator statsAggregator) throws HiveException, Met
parameters.putAll(providedBasicStats);
}
+ try {
+ long totalSize = 0L, numFiles = 0L;
+ // the stats are in the parameters already
+ String ts = parameters.get(StatsSetupConst.TOTAL_SIZE);
+ String nf = parameters.get(StatsSetupConst.NUM_FILES);
+ if (ts != null && nf != null) {
+ try {
+ totalSize = Long.parseLong(ts);
+ numFiles = Long.parseLong(nf);
+ } catch (NumberFormatException ignore) {
+ }
+ }
+ if (numFiles > 1 && totalSize > 0) {
+ long threshold = (conf != null)
+ ? conf.getLongVar(HiveConf.ConfVars.HIVE_MERGE_MAP_FILES_AVG_SIZE)
+ : HiveConf.ConfVars.HIVE_MERGE_MAP_FILES_AVG_SIZE.defaultLongVal;
+
+ long avg = totalSize / numFiles;
+ if (avg <= threshold) {
+ // both work for non-partitioned and partitioned tables
+ String who = (p.getPartition() == null)
+ ? ("table " + p.getTable().getFullyQualifiedName())
+ : ("partition " + p.getPartition().getName());
+
+ // add the small files warnings in the log
+ LOG.info("[ANALYZE] Small files detected: {} avgBytes={}, files={}, totalBytes={}",
Review Comment:
OK, changed to a case condition. If there is console, print to both console and log. If there is no console, only print in the logs.
--
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-28813: MSCK/Analyze commands can show a warning in console for Small files. [hive]
Posted by "DanielZhu58 (via GitHub)" <gi...@apache.org>.
DanielZhu58 commented on code in PR #5950:
URL: https://github.com/apache/hive/pull/5950#discussion_r2381324534
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreChecker.java:
##########
@@ -358,6 +361,28 @@ void checkTable(Table table, PartitionIterable parts, byte[] filterExp, CheckRes
if (partPath == null) {
continue;
}
+
+ // check the table/partition if (totalSize / numFiles) is less than avgFileSize
+ Map<String, String> partitionParameters = partition.getParameters();
+ if (partitionParameters != null) {
+ long totalSize = Long.parseLong(partitionParameters.getOrDefault(StatsSetupConst.TOTAL_SIZE, "0"));
Review Comment:
I think this part is 1 of 2 key-points of this PR. To provide end users verbal warnings when they do` msck` or `analyze`, about the existence of small files.
And the existence of small files will always be a case in the newly added partitions.
--
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-28813: MSCK/Analyze commands can show a warning in console for Small files. [hive]
Posted by "dengzhhu653 (via GitHub)" <gi...@apache.org>.
dengzhhu653 commented on code in PR #5950:
URL: https://github.com/apache/hive/pull/5950#discussion_r2437918716
##########
ql/src/java/org/apache/hadoop/hive/ql/stats/BasicStatsTask.java:
##########
@@ -182,6 +186,59 @@ public Object process(StatsAggregator statsAggregator) throws HiveException, Met
parameters.putAll(providedBasicStats);
}
+ try {
+ long totalSize = 0L, numFiles = 0L;
+ // Read stats from partition parameters only
+ // Parameters were already populated via populateQuickStats or providedBasicStats.putAll
+ String ts = parameters.get(StatsSetupConst.TOTAL_SIZE);
+ String nf = parameters.get(StatsSetupConst.NUM_FILES);
+
+ if (ts != null && nf != null) {
+ try {
+ totalSize = Long.parseLong(ts);
+ numFiles = Long.parseLong(nf);
+ } catch (NumberFormatException ignore) {
+ // If parsing fails, skip the warning without affecting the main flow
+ totalSize = 0L;
+ numFiles = 0L;
+ }
+ }
+
+ // Only warn when there are multiple files and total size > 0
+ if (numFiles > 100 && totalSize > 0) {
+ long threshold = (conf != null)
+ ? conf.getLongVar(HiveConf.ConfVars.HIVE_MERGE_MAP_FILES_AVG_SIZE)
+ : HiveConf.ConfVars.HIVE_MERGE_MAP_FILES_AVG_SIZE.defaultLongVal;
+
+ long avg = totalSize / numFiles;
+
+ if (avg <= threshold) {
Review Comment:
here we can also use
```
String who = (partish.getPartition() == null) ? ("table " + partish.getTable().getFullyQualifiedName())
: ("partition " + partish.getPartition().getName());
long threshold = conf.getLongVar(HiveConf.ConfVars.HIVE_MERGE_MAP_FILES_AVG_SIZE);
SmallFilesWarningUtil.smallFilesWarnings(parameters, 100L, threshold, who, "[ANALYZE]")
.ifPresent(msg -> {
LOG.warn(msg);
SessionState ss = SessionState.get();
if (ss != null && ss.getConsole() != null) {
ss.getConsole().printInfo(msg);
}
});
```
--
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-28813: MSCK/Analyze commands can show a warning in console for Small files. [hive]
Posted by "dengzhhu653 (via GitHub)" <gi...@apache.org>.
dengzhhu653 commented on code in PR #5950:
URL: https://github.com/apache/hive/pull/5950#discussion_r2437916607
##########
ql/src/java/org/apache/hadoop/hive/ql/stats/BasicStatsNoJobTask.java:
##########
@@ -181,6 +185,21 @@ public void run() {
}
parameters.putAll(basicStatistics);
StatsSetupConst.setBasicStatsState(parameters, StatsSetupConst.TRUE);
+ try {
+ String who = (partish.getPartition() == null) ? ("table " + partish.getTable().getFullyQualifiedName())
+ : ("partition " + partish.getPartition().getName());
+ long threshold = conf.getLongVar(HiveConf.ConfVars.HIVE_MERGE_MAP_FILES_AVG_SIZE);
+ SmallFilesWarningUtil.smallFilesWarnings(parameters, 100L, threshold, who, "[ANALYZE][NOSCAN]")
+ .ifPresent(msg -> {
+ LOG.info(msg);
+ SessionState ss = SessionState.get();
+ if (ss != null && ss.getConsole() != null) {
+ ss.getConsole().printInfo(msg);
+ }
+ });
+ } catch (Throwable t) {
Review Comment:
Is there any chance `SmallFilesWarningUtil.smallFilesWarnings` might throw the exception, if so why not catch `Throwable` in `SmallFilesWarningUtil.smallFilesWarnings`?
--
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-28813: MSCK/Analyze commands can show a warning in console for Small files. [hive]
Posted by "dengzhhu653 (via GitHub)" <gi...@apache.org>.
dengzhhu653 commented on code in PR #5950:
URL: https://github.com/apache/hive/pull/5950#discussion_r2443662914
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/Msck.java:
##########
@@ -240,6 +248,74 @@ public int repair(MsckInfo msckInfo) throws TException, MetastoreException, IOEx
}
}
+ // Generate small files warnings only for partsNotInMs
+ try {
+ // Threshold in bytes for average file size considered "small"
+ final long threshold =
+ MetastoreConf.getLongVar(getConf(), MetastoreConf.ConfVars.MSCK_SMALLFILES_AVG_SIZE);
+
+ // Collect partition names
+ final List<String> names = partsNotInMs.stream()
+ .map(CheckResult.PartitionResult::getPartitionName)
+ .filter(Objects::nonNull)
+ .filter(s -> !s.isEmpty())
+ .collect(Collectors.toList());
+
+ // Batch over the names to limit single-RPC payload.
+ final int PART_FETCH_BATCH = 1000;
+ final Map<String, Partition> byName = new HashMap<>(names.size() * 2);
+ List<Partition> allParts = Batchable.runBatched(
+ PART_FETCH_BATCH,
+ names,
+ new Batchable<String, Partition>() {
+ @Override
+ public List<Partition> run(List<String> batch) throws Exception {
+ final GetPartitionsByNamesRequest req =
+ new GetPartitionsByNamesRequest(table.getDbName(), table.getTableName());
+ req.setNames(batch);
+ try {
+ @SuppressWarnings("unchecked")
+ List<Partition> plist = (List<Partition>) getMsc().getPartitionsByNames(req);
+ return (plist != null) ? plist : Collections.emptyList();
+ } catch (NoSuchObjectException e) {
+ return Collections.emptyList();
+ }
+ }
+ });
+
+ for (Partition p : allParts) {
+ final String pName = Warehouse.makePartName(table.getPartitionKeys(), p.getValues());
+ byName.put(pName, p);
+ }
+
+ // Build small-files stats for partitions that have quick HMS stats.
+ final Map<String, String> smallFilesStats = new TreeMap<>();
+
+ for (String pName : names) {
+ final Partition p = byName.get(pName);
+ if (p == null) {
Review Comment:
remove the `p == null` check, this shouldn't be null
--
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-28813: MSCK/Analyze commands can show a warning in console for Small files. [hive]
Posted by "DanielZhu58 (via GitHub)" <gi...@apache.org>.
DanielZhu58 commented on code in PR #5950:
URL: https://github.com/apache/hive/pull/5950#discussion_r2443882184
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/Msck.java:
##########
@@ -240,6 +248,74 @@ public int repair(MsckInfo msckInfo) throws TException, MetastoreException, IOEx
}
}
+ // Generate small files warnings only for partsNotInMs
+ try {
+ // Threshold in bytes for average file size considered "small"
+ final long threshold =
+ MetastoreConf.getLongVar(getConf(), MetastoreConf.ConfVars.MSCK_SMALLFILES_AVG_SIZE);
+
+ // Collect partition names
+ final List<String> names = partsNotInMs.stream()
+ .map(CheckResult.PartitionResult::getPartitionName)
+ .filter(Objects::nonNull)
+ .filter(s -> !s.isEmpty())
+ .collect(Collectors.toList());
+
+ // Batch over the names to limit single-RPC payload.
+ final int PART_FETCH_BATCH = 1000;
+ final Map<String, Partition> byName = new HashMap<>(names.size() * 2);
+ List<Partition> allParts = Batchable.runBatched(
+ PART_FETCH_BATCH,
Review Comment:
Acknowledged.
--
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-28813: MSCK/Analyze commands can show a warning in console for Small files. [hive]
Posted by "dengzhhu653 (via GitHub)" <gi...@apache.org>.
dengzhhu653 commented on code in PR #5950:
URL: https://github.com/apache/hive/pull/5950#discussion_r2444077059
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/Msck.java:
##########
@@ -240,6 +248,68 @@ public int repair(MsckInfo msckInfo) throws TException, MetastoreException, IOEx
}
}
+ // Generate small files warnings only for partsNotInMs
+ try {
+ // Threshold in bytes for average file size considered "small"
+ final long threshold =
Review Comment:
remove `threshold`?
--
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-28813: MSCK/Analyze commands can show a warning in console for Small files. [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #5950:
URL: https://github.com/apache/hive/pull/5950#discussion_r2293469689
##########
ql/src/java/org/apache/hadoop/hive/ql/ddl/misc/msck/MsckOperation.java:
##########
@@ -75,7 +78,25 @@ public int execute() throws HiveException, IOException, TException, MetastoreExc
MsckInfo msckInfo = new MsckInfo(SessionState.get().getCurrentCatalog(), tableName.getDb(), tableName.getTable(),
desc.getFilterExp(), desc.getResFile(), desc.isRepairPartitions(),
desc.isAddPartitions(), desc.isDropPartitions(), partitionExpirySeconds);
- return msck.repair(msckInfo);
+ int result = msck.repair(msckInfo);
+ if (msckInfo.getSmallFilesStats() != null && !msckInfo.getSmallFilesStats().isEmpty()){
+ List<String> logInfo = new ArrayList<>(msckInfo.getSmallFilesStats().size());
+ for (Map.Entry<String, String> me : msckInfo.getSmallFilesStats().entrySet()){
+ String partitionName = me.getKey();
+ String smallFilesStats = me.getValue();
+ logInfo.add("This table/partition average file size is less than Hive average file size.\n " +
Review Comment:
should this be under warn?
##########
ql/src/java/org/apache/hadoop/hive/ql/ddl/misc/msck/MsckOperation.java:
##########
@@ -75,7 +78,25 @@ public int execute() throws HiveException, IOException, TException, MetastoreExc
MsckInfo msckInfo = new MsckInfo(SessionState.get().getCurrentCatalog(), tableName.getDb(), tableName.getTable(),
desc.getFilterExp(), desc.getResFile(), desc.isRepairPartitions(),
desc.isAddPartitions(), desc.isDropPartitions(), partitionExpirySeconds);
- return msck.repair(msckInfo);
+ int result = msck.repair(msckInfo);
+ if (msckInfo.getSmallFilesStats() != null && !msckInfo.getSmallFilesStats().isEmpty()){
+ List<String> logInfo = new ArrayList<>(msckInfo.getSmallFilesStats().size());
+ for (Map.Entry<String, String> me : msckInfo.getSmallFilesStats().entrySet()){
+ String partitionName = me.getKey();
+ String smallFilesStats = me.getValue();
+ logInfo.add("This table/partition average file size is less than Hive average file size.\n " +
+ "The partition name is " + partitionName + ". " + smallFilesStats);
+ }
+ SessionState ss = SessionState.get();
+ if (ss != null && ss.getConsole() != null) {
+ ss.getConsole().printInfo("[MSCK] Small files detected.");
+ for (String line : logInfo) {
+ ss.getConsole().printInfo("[MSCK] " + line);
+ }
+ }
+ LOG.info("There are small files exist.\n{}", String.join("\n", logInfo));
Review Comment:
should this be under warn?
--
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-28813: MSCK/Analyze commands can show a warning in console for Small files. [hive]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #5950:
URL: https://github.com/apache/hive/pull/5950#issuecomment-3420333265
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5950) **Quality Gate passed**
Issues
 [68 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5950&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5950&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=5950&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5950&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5950&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5950)
--
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-28813: MSCK/Analyze commands can show a warning in console for Small files. [hive]
Posted by "DanielZhu58 (via GitHub)" <gi...@apache.org>.
DanielZhu58 commented on code in PR #5950:
URL: https://github.com/apache/hive/pull/5950#discussion_r2443975560
##########
ql/src/java/org/apache/hadoop/hive/ql/stats/BasicStatsNoJobTask.java:
##########
@@ -181,6 +186,15 @@ public void run() {
}
parameters.putAll(basicStatistics);
StatsSetupConst.setBasicStatsState(parameters, StatsSetupConst.TRUE);
+ try {
+ String who = (partish.getPartition() == null) ? ("table " + partish.getTable().getFullyQualifiedName())
+ : ("partition " + partish.getPartition().getName());
+ long threshold = conf.getLongVar(HiveConf.ConfVars.HIVE_MERGE_MAP_FILES_AVG_SIZE);
+ SmallFilesWarningUtil.smallFilesWarnings(parameters, 100L, threshold, who, "[ANALYZE][NOSCAN]")
+ .ifPresent(smallFilesConsoleAndWarn(LOG));
+ } catch (Exception e) {
Review Comment:
Acknowledged.
--
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-28813: MSCK/Analyze commands can show a warning in console for Small files. [hive]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #5950:
URL: https://github.com/apache/hive/pull/5950#issuecomment-3054650675
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5950) **Quality Gate passed**
Issues
 [5 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5950&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5950&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=5950&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5950&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5950&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5950)
--
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-28813: MSCK/Analyze commands can show a warning in console for Small files. [hive]
Posted by "DanielZhu58 (via GitHub)" <gi...@apache.org>.
DanielZhu58 commented on code in PR #5950:
URL: https://github.com/apache/hive/pull/5950#discussion_r2443604046
##########
ql/src/java/org/apache/hadoop/hive/ql/stats/BasicStatsNoJobTask.java:
##########
@@ -181,6 +185,21 @@ public void run() {
}
parameters.putAll(basicStatistics);
StatsSetupConst.setBasicStatsState(parameters, StatsSetupConst.TRUE);
+ try {
Review Comment:
Acknowledged. A new static method about printing info is added in BasicStatsNoJobTask.java.
--
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-28813: MSCK/Analyze commands can show a warning in console for Small files. [hive]
Posted by "DanielZhu58 (via GitHub)" <gi...@apache.org>.
DanielZhu58 commented on code in PR #5950:
URL: https://github.com/apache/hive/pull/5950#discussion_r2444106356
##########
ql/src/java/org/apache/hadoop/hive/ql/stats/BasicStatsTask.java:
##########
@@ -182,6 +187,27 @@ public Object process(StatsAggregator statsAggregator) throws HiveException, Met
parameters.putAll(providedBasicStats);
}
+ try {
+ final long threshold = (conf != null)
+ ? conf.getLongVar(HiveConf.ConfVars.HIVE_MERGE_MAP_FILES_AVG_SIZE)
+ : HiveConf.ConfVars.HIVE_MERGE_MAP_FILES_AVG_SIZE.defaultLongVal;
+
+ final String who = (p.getPartition() == null)
+ ? "table " + p.getTable().getFullyQualifiedName()
+ : "partition " + p.getPartition().getName();
+
+ SmallFilesWarningUtil.smallFilesWarnings(parameters, 100L, threshold, who, "[ANALYZE]")
+ .ifPresent(msg -> {
+ LOG.warn(msg);
+ SessionState ss = SessionState.get();
+ if (ss != null && ss.getConsole() != null) {
+ ss.getConsole().printInfo(msg);
+ }
+ });
+ } catch (Exception e) {
Review Comment:
Acknowledged.
--
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-28813: MSCK/Analyze commands can show a warning in console for Small files. [hive]
Posted by "dengzhhu653 (via GitHub)" <gi...@apache.org>.
dengzhhu653 merged PR #5950:
URL: https://github.com/apache/hive/pull/5950
--
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-28813: MSCK/Analyze commands can show a warning in console for Small files. [hive]
Posted by "dengzhhu653 (via GitHub)" <gi...@apache.org>.
dengzhhu653 commented on code in PR #5950:
URL: https://github.com/apache/hive/pull/5950#discussion_r2371007808
##########
ql/src/java/org/apache/hadoop/hive/ql/stats/BasicStatsTask.java:
##########
@@ -182,6 +184,48 @@ public Object process(StatsAggregator statsAggregator) throws HiveException, Met
parameters.putAll(providedBasicStats);
}
+ try {
+ long totalSize = 0L, numFiles = 0L;
+ // the stats are in the parameters already
+ String ts = parameters.get(StatsSetupConst.TOTAL_SIZE);
+ String nf = parameters.get(StatsSetupConst.NUM_FILES);
+ if (ts != null && nf != null) {
+ try {
+ totalSize = Long.parseLong(ts);
+ numFiles = Long.parseLong(nf);
+ } catch (NumberFormatException ignore) {
+ }
+ }
+ if (numFiles > 1 && totalSize > 0) {
+ long threshold = (conf != null)
+ ? conf.getLongVar(HiveConf.ConfVars.HIVE_MERGE_MAP_FILES_AVG_SIZE)
+ : HiveConf.ConfVars.HIVE_MERGE_MAP_FILES_AVG_SIZE.defaultLongVal;
+
+ long avg = totalSize / numFiles;
+ if (avg <= threshold) {
+ // both work for non-partitioned and partitioned tables
+ String who = (p.getPartition() == null)
+ ? ("table " + p.getTable().getFullyQualifiedName())
+ : ("partition " + p.getPartition().getName());
+
+ // add the small files warnings in the log
+ LOG.info("[ANALYZE] Small files detected: {} avgBytes={}, files={}, totalBytes={}",
Review Comment:
this message will be logged as well in case ss != null && ss.getConsole() != null, which may lead to duplication in the log file
--
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-28813: MSCK/Analyze commands can show a warning in console for Small files. [hive]
Posted by "dengzhhu653 (via GitHub)" <gi...@apache.org>.
dengzhhu653 commented on code in PR #5950:
URL: https://github.com/apache/hive/pull/5950#discussion_r2371017645
##########
ql/src/java/org/apache/hadoop/hive/ql/stats/BasicStatsNoJobTask.java:
##########
@@ -181,6 +184,35 @@ public void run() {
}
parameters.putAll(basicStatistics);
StatsSetupConst.setBasicStatsState(parameters, StatsSetupConst.TRUE);
+ try {
+ String ts = parameters.get(StatsSetupConst.TOTAL_SIZE);
Review Comment:
the code is almost the same as `BasicStatsTask`, can we able to make it a static util method
--
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-28813: MSCK/Analyze commands can show a warning in console for Small files. [hive]
Posted by "dengzhhu653 (via GitHub)" <gi...@apache.org>.
dengzhhu653 commented on code in PR #5950:
URL: https://github.com/apache/hive/pull/5950#discussion_r2371022128
##########
ql/src/java/org/apache/hadoop/hive/ql/stats/BasicStatsNoJobTask.java:
##########
@@ -293,6 +325,28 @@ public void run() {
parameters.put(StatsSetupConst.NUM_FILES, String.valueOf(numFiles));
parameters.put(StatsSetupConst.NUM_ERASURE_CODED_FILES, String.valueOf(numErasureCodedFiles));
+ try {
+ if (numFiles > 1 && fileSize > 0) {
Review Comment:
why again in the same class
--
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-28813: MSCK/Analyze commands can show a warning in console for Small files. [hive]
Posted by "DanielZhu58 (via GitHub)" <gi...@apache.org>.
DanielZhu58 commented on code in PR #5950:
URL: https://github.com/apache/hive/pull/5950#discussion_r2381335235
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/Msck.java:
##########
@@ -239,6 +244,57 @@ public int repair(MsckInfo msckInfo) throws TException, MetastoreException, IOEx
throw new MetastoreException(e);
}
}
+ // Generate small files warnings only for partsNotInMs
+ try {
+ long threshold = HiveConf.getLongVar(getConf(), HiveConf.ConfVars.HIVE_MERGE_MAP_FILES_AVG_SIZE);
+ Map<String, String> smallFilesStats = new TreeMap<>();
+
+ for (CheckResult.PartitionResult pr : partsNotInMs) {
+ List<String> statsVals;
+ try {
+ // parse the partition name into a few partition values
+ statsVals = Warehouse.makeValsFromName(pr.getPartitionName(), null);
+ } catch (MetaException me) {
+ // skip this partition if this somehow failed
+ continue;
+ }
+
+ Partition p;
+ try {
+ // the newly added partition should exist in metastore
+ p = getMsc().getPartition(table.getCatName(), table.getDbName(), table.getTableName(), statsVals);
+ } catch (NoSuchObjectException nsoe) {
+ // the partition is not actually added in metastore
+ continue;
+ }
+
+ Path loc = new Path(p.getSd().getLocation());
+ try {
+ FileSystem fs = loc.getFileSystem(getConf());
Review Comment:
After discussed with Naresh, the msck command can compute the basic stats, including the total size and total amount of files.
So we don't need the file system params anymore currently.
--
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-28813: MSCK/Analyze commands can show a warning in console for Small files. [hive]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #5950:
URL: https://github.com/apache/hive/pull/5950#issuecomment-3191372168
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5950) **Quality Gate passed**
Issues
 [29 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5950&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5950&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=5950&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5950&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5950&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5950)
--
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-28813: MSCK/Analyze commands can show a warning in console for Small files. [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #5950:
URL: https://github.com/apache/hive/pull/5950#discussion_r2201495706
##########
ql/src/java/org/apache/hadoop/hive/ql/ddl/misc/msck/MsckOperation.java:
##########
@@ -75,7 +78,18 @@ public int execute() throws HiveException, IOException, TException, MetastoreExc
MsckInfo msckInfo = new MsckInfo(SessionState.get().getCurrentCatalog(), tableName.getDb(), tableName.getTable(),
desc.getFilterExp(), desc.getResFile(), desc.isRepairPartitions(),
desc.isAddPartitions(), desc.isDropPartitions(), partitionExpirySeconds);
- return msck.repair(msckInfo);
+ int result = msck.repair(msckInfo);
+ if (msckInfo.getSmallFilesStats() != null && !msckInfo.getSmallFilesStats().isEmpty()){
Review Comment:
can we log the warning in `msck.repair` while processing?
--
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-28813: MSCK/Analyze commands can show a warning in console for Small files. [hive]
Posted by "jameskramden (via GitHub)" <gi...@apache.org>.
jameskramden commented on code in PR #5950:
URL: https://github.com/apache/hive/pull/5950#discussion_r2314159271
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreChecker.java:
##########
@@ -358,6 +361,28 @@ void checkTable(Table table, PartitionIterable parts, byte[] filterExp, CheckRes
if (partPath == null) {
continue;
}
+
+ // check the table/partition if (totalSize / numFiles) is less than avgFileSize
+ Map<String, String> partitionParameters = partition.getParameters();
+ if (partitionParameters != null) {
+ long totalSize = Long.parseLong(partitionParameters.getOrDefault(StatsSetupConst.TOTAL_SIZE, "0"));
+ long numFiles = Long.parseLong(partitionParameters.getOrDefault(StatsSetupConst.NUM_FILES, "0"));
+ if (numFiles != 0) {
+ long avgFileSize = totalSize / numFiles;
+ long HIVE_FILES_AVG_SIZE = conf.getLong("hive.merge.smallfiles.avgsize", 0);
+ if (avgFileSize <= HIVE_FILES_AVG_SIZE) {
+ StringBuilder tmpStatsSb = new StringBuilder();
+ tmpStatsSb.append("totalSize = ").append(totalSize).append(", numFiles = ")
+ .append(numFiles).append(". ");
+ smallFilesStats.putIfAbsent(Warehouse.makePartName(table.getPartitionKeys(), partition.getValues()), tmpStatsSb.toString());
+ result.setSmallFilesStats(smallFilesStats);
+ }
+ } else {
+ LOG.warn("Partition total number of files is 0.");
Review Comment:
This line logs a warning message that a partition has 0 files. This is bad because it doesn't add information which partition has 0 files. Log may contain several warnings that a partition has 0 files and the person who reviews the log won't know which partitions are empty.
--
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-28813: MSCK/Analyze commands can show a warning in console for Small files. [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #5950:
URL: https://github.com/apache/hive/pull/5950#discussion_r2201499725
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/MsckInfo.java:
##########
@@ -33,6 +36,8 @@ public class MsckInfo {
private final boolean dropPartitions;
private final long partitionExpirySeconds;
+ private Map<String, String> smallFilesStats = new HashMap<>();
Review Comment:
it can be costly in terms of memory. isn't simple counter enough here?
--
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-28813: MSCK/Analyze commands can show a warning in console for Small files. [hive]
Posted by "DanielZhu58 (via GitHub)" <gi...@apache.org>.
DanielZhu58 commented on code in PR #5950:
URL: https://github.com/apache/hive/pull/5950#discussion_r2401263787
##########
ql/src/java/org/apache/hadoop/hive/ql/ddl/misc/msck/MsckOperation.java:
##########
@@ -75,7 +78,25 @@ public int execute() throws HiveException, IOException, TException, MetastoreExc
MsckInfo msckInfo = new MsckInfo(SessionState.get().getCurrentCatalog(), tableName.getDb(), tableName.getTable(),
desc.getFilterExp(), desc.getResFile(), desc.isRepairPartitions(),
desc.isAddPartitions(), desc.isDropPartitions(), partitionExpirySeconds);
- return msck.repair(msckInfo);
+ int result = msck.repair(msckInfo);
+ Map<String, String> smallFilesStats = msckInfo.getSmallFilesStats();
+ if (smallFilesStats != null && !smallFilesStats.isEmpty()) {
+ // keep the small files information in the logs
+ List<String> logInfo = smallFilesStats.entrySet().stream()
+ .map(entry -> String.format(
+ "This table/partition average file size is less than Hive average file size.%n The partition name is %s. %s",
Review Comment:
Acknowledged.
--
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-28813: MSCK/Analyze commands can show a warning in console for Small files. [hive]
Posted by "DanielZhu58 (via GitHub)" <gi...@apache.org>.
DanielZhu58 commented on code in PR #5950:
URL: https://github.com/apache/hive/pull/5950#discussion_r2321526055
##########
ql/src/java/org/apache/hadoop/hive/ql/ddl/misc/msck/MsckOperation.java:
##########
@@ -75,7 +78,18 @@ public int execute() throws HiveException, IOException, TException, MetastoreExc
MsckInfo msckInfo = new MsckInfo(SessionState.get().getCurrentCatalog(), tableName.getDb(), tableName.getTable(),
desc.getFilterExp(), desc.getResFile(), desc.isRepairPartitions(),
desc.isAddPartitions(), desc.isDropPartitions(), partitionExpirySeconds);
- return msck.repair(msckInfo);
+ int result = msck.repair(msckInfo);
+ if (msckInfo.getSmallFilesStats() != null && !msckInfo.getSmallFilesStats().isEmpty()){
Review Comment:
I would prefer to not add the "small files warning" in `msck.repair()` function.
The purpose of repair() will be only plan and apply metadata fixes, user messaging can be belong to the check/plan stage and the CLI layer (MsckOperation).
--
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-28813: MSCK/Analyze commands can show a warning in console for Small files. [hive]
Posted by "dengzhhu653 (via GitHub)" <gi...@apache.org>.
dengzhhu653 commented on code in PR #5950:
URL: https://github.com/apache/hive/pull/5950#discussion_r2437801252
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/Msck.java:
##########
@@ -240,6 +246,76 @@ public int repair(MsckInfo msckInfo) throws TException, MetastoreException, IOEx
}
}
+ // Generate small files warnings only for partsNotInMs
+ try {
+ // Threshold in bytes for average file size considered "small"
+ final long threshold =
+ MetastoreConf.getLongVar(getConf(), MetastoreConf.ConfVars.MSCK_SMALLFILES_AVG_SIZE);
+
+ // Collect partition names
+ final List<String> names = new ArrayList<>(partsNotInMs.size());
+ for (CheckResult.PartitionResult pr : partsNotInMs) {
Review Comment:
final List<String> names = partsNotInMs.stream()...
--
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-28813: MSCK/Analyze commands can show a warning in console for Small files. [hive]
Posted by "dengzhhu653 (via GitHub)" <gi...@apache.org>.
dengzhhu653 commented on code in PR #5950:
URL: https://github.com/apache/hive/pull/5950#discussion_r2430915800
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/Msck.java:
##########
@@ -240,6 +244,61 @@ public int repair(MsckInfo msckInfo) throws TException, MetastoreException, IOEx
}
}
+ // Generate small files warnings only for partsNotInMs
+ try {
+ long threshold = MetastoreConf.getLongVar(getConf(), MetastoreConf.ConfVars.MSCK_SMALLFILES_AVG_SIZE);
+ Map<String, String> smallFilesStats = new TreeMap<>();
+ for (CheckResult.PartitionResult pr : partsNotInMs) {
+ List<String> statsVals;
+ try {
+ // parse the partition name into a few partition values
+ statsVals = Warehouse.makeValsFromName(pr.getPartitionName(), null);
+ } catch (MetaException me) {
+ // skip this partition if this somehow failed
+ continue;
+ }
+ Partition p;
+ try {
+ // the newly added partition should exist in metastore
+ p = getMsc().getPartition(table.getCatName(), table.getDbName(), table.getTableName(), statsVals);
Review Comment:
instead of retrieving the partition one by one, can we use `msc.getPartitionsByNames`?
--
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-28813: MSCK/Analyze commands can show a warning in console for Small files. [hive]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #5950:
URL: https://github.com/apache/hive/pull/5950#issuecomment-3272565168
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5950) **Quality Gate passed**
Issues
 [11 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5950&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5950&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=5950&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5950&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5950&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5950)
--
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-28813: MSCK/Analyze commands can show a warning in console for Small files. [hive]
Posted by "DanielZhu58 (via GitHub)" <gi...@apache.org>.
DanielZhu58 commented on code in PR #5950:
URL: https://github.com/apache/hive/pull/5950#discussion_r2381299103
##########
ql/src/java/org/apache/hadoop/hive/ql/stats/BasicStatsNoJobTask.java:
##########
@@ -293,6 +325,28 @@ public void run() {
parameters.put(StatsSetupConst.NUM_FILES, String.valueOf(numFiles));
parameters.put(StatsSetupConst.NUM_ERASURE_CODED_FILES, String.valueOf(numErasureCodedFiles));
+ try {
+ if (numFiles > 1 && fileSize > 0) {
Review Comment:
Because ANALYZE NOSCAN has two different code paths:
Storage-handler path (HiveStorageHandlerStatCollector): stats come from the storage handler.
Footer path (FooterStatCollector): stats are computed from ORC/Parquet footers.
--
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-28813: MSCK/Analyze commands can show a warning in console for Small files. [hive]
Posted by "DanielZhu58 (via GitHub)" <gi...@apache.org>.
DanielZhu58 commented on code in PR #5950:
URL: https://github.com/apache/hive/pull/5950#discussion_r2443574916
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/Msck.java:
##########
@@ -240,6 +246,76 @@ public int repair(MsckInfo msckInfo) throws TException, MetastoreException, IOEx
}
}
+ // Generate small files warnings only for partsNotInMs
+ try {
+ // Threshold in bytes for average file size considered "small"
+ final long threshold =
+ MetastoreConf.getLongVar(getConf(), MetastoreConf.ConfVars.MSCK_SMALLFILES_AVG_SIZE);
+
+ // Collect partition names
+ final List<String> names = new ArrayList<>(partsNotInMs.size());
+ for (CheckResult.PartitionResult pr : partsNotInMs) {
+ final String name = pr.getPartitionName();
+ if (name != null && !name.isEmpty()) {
+ names.add(name);
+ }
+ }
+
+ // Batch over the names to limit single-RPC payload.
+ final int BATCH = 1000;
+ final Map<String, Partition> byName = new HashMap<>(names.size() * 2);
+
+ for (int i = 0; i < names.size(); i += BATCH) {
+ final List<String> batch = names.subList(i, Math.min(i + BATCH, names.size()));
+ final GetPartitionsByNamesRequest req = new GetPartitionsByNamesRequest(table.getDbName(), table.getTableName());
+ req.setNames(batch);
+
+ // In this branch, getPartitionsByNames returns List<Partition>
+ List<Partition> plist;
+ try {
+ @SuppressWarnings("unchecked")
+ List<Partition> tmp = (List<Partition>) getMsc().getPartitionsByNames(req);
+ plist = (tmp != null) ? tmp : Collections.emptyList();
+ } catch (NoSuchObjectException e) {
+ plist = Collections.emptyList();
+ }
+
+ for (Partition p : plist) {
+ final String pName = Warehouse.makePartName(table.getPartitionKeys(), p.getValues());
+ byName.put(pName, p);
+ }
+ }
+
+ // Build small-files stats for partitions that have quick HMS stats.
+ final Map<String, String> smallFilesStats = new TreeMap<>();
+
+ for (String pName : names) {
+ final Partition p = byName.get(pName);
+ if (p == null) {
+ // not found / not visible yet
+ continue;
+ }
+
+ final Map<String, String> params = p.getParameters();
+ // Use util to decide if this partition should trigger the warning
+ if (SmallFilesWarningUtil.smallAverageFilesDetected(SmallFilesWarningUtil.DEFAULT_MIN_FILES, threshold, params)) {
+ // We know stats exist and are well-formed; compute numbers for the value string
+ final long totalSize = Long.parseLong(params.get(StatsSetupConst.TOTAL_SIZE));
+ final long numFiles = Long.parseLong(params.get(StatsSetupConst.NUM_FILES));
+ final long avg = Math.floorDiv(totalSize, numFiles);
+ smallFilesStats.put(pName, "avgBytes=" + avg + ", partition total files=" + numFiles + ", totalBytes=" + totalSize);
+ }
+ }
+
+ msckInfo.setSmallFilesStats(smallFilesStats.isEmpty() ? Collections.emptyMap() : smallFilesStats);
Review Comment:
Acknowledged.
--
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-28813: MSCK/Analyze commands can show a warning in console for Small files. [hive]
Posted by "dengzhhu653 (via GitHub)" <gi...@apache.org>.
dengzhhu653 commented on code in PR #5950:
URL: https://github.com/apache/hive/pull/5950#discussion_r2443659032
##########
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/utils/SmallFilesWarningUtil.java:
##########
@@ -0,0 +1,104 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hive.metastore.utils;
+
+import java.util.Map;
+import java.util.Optional;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hive.common.StatsSetupConst;
+import org.apache.hadoop.hive.metastore.conf.MetastoreConf;
+
+public final class SmallFilesWarningUtil {
+ private SmallFilesWarningUtil() {}
+
+ /** Default minimum number of files before we consider emitting the warning. */
+ public static final long DEFAULT_MIN_FILES = 100L;
+
+ /**
+ * True if quick HMS stats in {@code parameters} indicate small average file size.
+ * Missing/malformed stats or insufficient file count return false.
+ */
+ public static boolean smallAverageFilesDetected(long minFiles,
+ long avgSizeThreshold,
+ Map<String, String> parameters) {
+ if (parameters == null) return false;
+
+ final String ts = parameters.get(StatsSetupConst.TOTAL_SIZE);
+ final String nf = parameters.get(StatsSetupConst.NUM_FILES);
+ if (ts == null || nf == null) return false;
+
+ final long totalSize;
+ final long numFiles;
+ try {
+ totalSize = Long.parseLong(ts);
+ numFiles = Long.parseLong(nf);
+ } catch (NumberFormatException ignore) {
+ return false;
+ }
+
+ if (numFiles <= minFiles || totalSize <= 0) return false;
+
+ final long avg = Math.floorDiv(totalSize, numFiles);
+ return avg <= avgSizeThreshold;
+ }
+
+ /**
+ * Convenience overload: reads the threshold from {@code conf} and uses {@link #DEFAULT_MIN_FILES}.
+ * Returns false if {@code conf} is null.
+ */
+ public static boolean smallAverageFilesDetected(Configuration conf,
+ Map<String, String> parameters) {
+ if (conf == null) return false;
Review Comment:
why `conf` would be null?
--
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-28813: MSCK/Analyze commands can show a warning in console for Small files. [hive]
Posted by "dengzhhu653 (via GitHub)" <gi...@apache.org>.
dengzhhu653 commented on code in PR #5950:
URL: https://github.com/apache/hive/pull/5950#discussion_r2443661814
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/Msck.java:
##########
@@ -240,6 +248,74 @@ public int repair(MsckInfo msckInfo) throws TException, MetastoreException, IOEx
}
}
+ // Generate small files warnings only for partsNotInMs
+ try {
+ // Threshold in bytes for average file size considered "small"
+ final long threshold =
+ MetastoreConf.getLongVar(getConf(), MetastoreConf.ConfVars.MSCK_SMALLFILES_AVG_SIZE);
+
+ // Collect partition names
+ final List<String> names = partsNotInMs.stream()
+ .map(CheckResult.PartitionResult::getPartitionName)
+ .filter(Objects::nonNull)
+ .filter(s -> !s.isEmpty())
+ .collect(Collectors.toList());
+
+ // Batch over the names to limit single-RPC payload.
+ final int PART_FETCH_BATCH = 1000;
+ final Map<String, Partition> byName = new HashMap<>(names.size() * 2);
+ List<Partition> allParts = Batchable.runBatched(
+ PART_FETCH_BATCH,
+ names,
+ new Batchable<String, Partition>() {
+ @Override
+ public List<Partition> run(List<String> batch) throws Exception {
+ final GetPartitionsByNamesRequest req =
+ new GetPartitionsByNamesRequest(table.getDbName(), table.getTableName());
+ req.setNames(batch);
+ try {
+ @SuppressWarnings("unchecked")
+ List<Partition> plist = (List<Partition>) getMsc().getPartitionsByNames(req);
+ return (plist != null) ? plist : Collections.emptyList();
+ } catch (NoSuchObjectException e) {
+ return Collections.emptyList();
+ }
+ }
+ });
+
+ for (Partition p : allParts) {
+ final String pName = Warehouse.makePartName(table.getPartitionKeys(), p.getValues());
+ byName.put(pName, p);
+ }
+
+ // Build small-files stats for partitions that have quick HMS stats.
+ final Map<String, String> smallFilesStats = new TreeMap<>();
+
+ for (String pName : names) {
+ final Partition p = byName.get(pName);
+ if (p == null) {
+ // not found / not visible yet
+ continue;
+ }
+
+ final Map<String, String> params = p.getParameters();
+ // Use util to decide if this partition should trigger the warning
+ if (SmallFilesWarningUtil.smallAverageFilesDetected(SmallFilesWarningUtil.DEFAULT_MIN_FILES, threshold, params)) {
Review Comment:
nit: why not `SmallFilesWarningUtil. smallAverageFilesDetected(conf, params)`?
--
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-28813: MSCK/Analyze commands can show a warning in console for Small files. [hive]
Posted by "jameskramden (via GitHub)" <gi...@apache.org>.
jameskramden commented on code in PR #5950:
URL: https://github.com/apache/hive/pull/5950#discussion_r2314159271
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreChecker.java:
##########
@@ -358,6 +361,28 @@ void checkTable(Table table, PartitionIterable parts, byte[] filterExp, CheckRes
if (partPath == null) {
continue;
}
+
+ // check the table/partition if (totalSize / numFiles) is less than avgFileSize
+ Map<String, String> partitionParameters = partition.getParameters();
+ if (partitionParameters != null) {
+ long totalSize = Long.parseLong(partitionParameters.getOrDefault(StatsSetupConst.TOTAL_SIZE, "0"));
+ long numFiles = Long.parseLong(partitionParameters.getOrDefault(StatsSetupConst.NUM_FILES, "0"));
+ if (numFiles != 0) {
+ long avgFileSize = totalSize / numFiles;
+ long HIVE_FILES_AVG_SIZE = conf.getLong("hive.merge.smallfiles.avgsize", 0);
+ if (avgFileSize <= HIVE_FILES_AVG_SIZE) {
+ StringBuilder tmpStatsSb = new StringBuilder();
+ tmpStatsSb.append("totalSize = ").append(totalSize).append(", numFiles = ")
+ .append(numFiles).append(". ");
+ smallFilesStats.putIfAbsent(Warehouse.makePartName(table.getPartitionKeys(), partition.getValues()), tmpStatsSb.toString());
+ result.setSmallFilesStats(smallFilesStats);
+ }
+ } else {
+ LOG.warn("Partition total number of files is 0.");
Review Comment:
This line logs a warning message that a partition has 0 files. This is bad because it doesn't add information which partition has 0 files. Log may contain several warnings that a partition has 0 files and the person who reviews the log won't know which partitions are empty.
--
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-28813: MSCK/Analyze commands can show a warning in console for Small files. [hive]
Posted by "dengzhhu653 (via GitHub)" <gi...@apache.org>.
dengzhhu653 commented on code in PR #5950:
URL: https://github.com/apache/hive/pull/5950#discussion_r2348114485
##########
ql/src/java/org/apache/hadoop/hive/ql/exec/StatsTask.java:
##########
@@ -124,6 +133,125 @@ public int execute() {
return 0;
}
+ /**
+ * Check for small files after ANALYZE completes
+ */
+ private void checkSmallFilesAfterAnalyze(Hive db, Table tbl) {
+ // Use the existing merge small files threshold configuration
+ long smallFilesThreshold = conf.getLongVar(HiveConf.ConfVars.HIVE_MERGE_MAP_FILES_AVG_SIZE);
+ List<String> smallFilesWarnings = new ArrayList<>();
+
+ // Get the console from SessionState for output
+ PrintStream console = SessionState.get().out;
Review Comment:
`SessionState.get().out` for printing the message might not be viewable in the Beeline? use `SessionState.get().getConsole()`?
--
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-28813: MSCK/Analyze commands can show a warning in console for Small files. [hive]
Posted by "dengzhhu653 (via GitHub)" <gi...@apache.org>.
dengzhhu653 commented on code in PR #5950:
URL: https://github.com/apache/hive/pull/5950#discussion_r2371010765
##########
ql/src/java/org/apache/hadoop/hive/ql/ddl/misc/msck/MsckOperation.java:
##########
@@ -75,7 +78,25 @@ public int execute() throws HiveException, IOException, TException, MetastoreExc
MsckInfo msckInfo = new MsckInfo(SessionState.get().getCurrentCatalog(), tableName.getDb(), tableName.getTable(),
desc.getFilterExp(), desc.getResFile(), desc.isRepairPartitions(),
desc.isAddPartitions(), desc.isDropPartitions(), partitionExpirySeconds);
- return msck.repair(msckInfo);
+ int result = msck.repair(msckInfo);
+ Map<String, String> smallFilesStats = msckInfo.getSmallFilesStats();
+ if (smallFilesStats != null && !smallFilesStats.isEmpty()) {
+ // keep the small files information in the logs
+ List<String> logInfo = smallFilesStats.entrySet().stream()
+ .map(entry -> String.format(
+ "This table/partition average file size is less than Hive average file size.%n The partition name is %s. %s",
Review Comment:
lots of the same message might flush the client, we need to shorten the message per line
--
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-28813: MSCK/Analyze commands can show a warning in console for Small files. [hive]
Posted by "DanielZhu58 (via GitHub)" <gi...@apache.org>.
DanielZhu58 commented on code in PR #5950:
URL: https://github.com/apache/hive/pull/5950#discussion_r2381361226
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/Msck.java:
##########
@@ -239,6 +244,57 @@ public int repair(MsckInfo msckInfo) throws TException, MetastoreException, IOEx
throw new MetastoreException(e);
}
}
+ // Generate small files warnings only for partsNotInMs
+ try {
+ long threshold = HiveConf.getLongVar(getConf(), HiveConf.ConfVars.HIVE_MERGE_MAP_FILES_AVG_SIZE);
+ Map<String, String> smallFilesStats = new TreeMap<>();
+
+ for (CheckResult.PartitionResult pr : partsNotInMs) {
+ List<String> statsVals;
+ try {
+ // parse the partition name into a few partition values
+ statsVals = Warehouse.makeValsFromName(pr.getPartitionName(), null);
+ } catch (MetaException me) {
+ // skip this partition if this somehow failed
+ continue;
+ }
+
+ Partition p;
+ try {
+ // the newly added partition should exist in metastore
+ p = getMsc().getPartition(table.getCatName(), table.getDbName(), table.getTableName(), statsVals);
+ } catch (NoSuchObjectException nsoe) {
+ // the partition is not actually added in metastore
+ continue;
+ }
+
+ Path loc = new Path(p.getSd().getLocation());
Review Comment:
I think we don't need this anymore if we changed the implementation approach to using the partition parameters.
--
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-28813: MSCK/Analyze commands can show a warning in console for Small files. [hive]
Posted by "dengzhhu653 (via GitHub)" <gi...@apache.org>.
dengzhhu653 commented on code in PR #5950:
URL: https://github.com/apache/hive/pull/5950#discussion_r2443659891
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/Msck.java:
##########
@@ -240,6 +248,74 @@ public int repair(MsckInfo msckInfo) throws TException, MetastoreException, IOEx
}
}
+ // Generate small files warnings only for partsNotInMs
+ try {
+ // Threshold in bytes for average file size considered "small"
+ final long threshold =
+ MetastoreConf.getLongVar(getConf(), MetastoreConf.ConfVars.MSCK_SMALLFILES_AVG_SIZE);
+
+ // Collect partition names
+ final List<String> names = partsNotInMs.stream()
+ .map(CheckResult.PartitionResult::getPartitionName)
+ .filter(Objects::nonNull)
+ .filter(s -> !s.isEmpty())
+ .collect(Collectors.toList());
+
+ // Batch over the names to limit single-RPC payload.
+ final int PART_FETCH_BATCH = 1000;
+ final Map<String, Partition> byName = new HashMap<>(names.size() * 2);
+ List<Partition> allParts = Batchable.runBatched(
+ PART_FETCH_BATCH,
Review Comment:
PART_FETCH_BATCH -> `MetastoreConf.getIntVar(conf, MetastoreConf.ConfVars.BATCH_RETRIEVE_MAX)`
--
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-28813: MSCK/Analyze commands can show a warning in console for Small files. [hive]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #5950:
URL: https://github.com/apache/hive/pull/5950#issuecomment-3440956059
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5950) **Quality Gate passed**
Issues
 [63 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5950&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5950&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=5950&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5950&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5950&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5950)
--
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-28813: MSCK/Analyze commands can show a warning in console for Small files. [hive]
Posted by "DanielZhu58 (via GitHub)" <gi...@apache.org>.
DanielZhu58 commented on code in PR #5950:
URL: https://github.com/apache/hive/pull/5950#discussion_r2401265844
##########
ql/src/java/org/apache/hadoop/hive/ql/ddl/misc/msck/MsckOperation.java:
##########
@@ -75,7 +78,25 @@ public int execute() throws HiveException, IOException, TException, MetastoreExc
MsckInfo msckInfo = new MsckInfo(SessionState.get().getCurrentCatalog(), tableName.getDb(), tableName.getTable(),
desc.getFilterExp(), desc.getResFile(), desc.isRepairPartitions(),
desc.isAddPartitions(), desc.isDropPartitions(), partitionExpirySeconds);
- return msck.repair(msckInfo);
+ int result = msck.repair(msckInfo);
+ Map<String, String> smallFilesStats = msckInfo.getSmallFilesStats();
+ if (smallFilesStats != null && !smallFilesStats.isEmpty()) {
+ // keep the small files information in the logs
+ List<String> logInfo = smallFilesStats.entrySet().stream()
+ .map(entry -> String.format(
+ "This table/partition average file size is less than Hive average file size.%n The partition name is %s. %s",
+ entry.getKey(), entry.getValue()))
+ .collect(Collectors.toList());
+ LOG.info("There are small files exist.\n{}", String.join("\n", logInfo));
Review Comment:
Acknowledged.
--
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-28813: MSCK/Analyze commands can show a warning in console for Small files. [hive]
Posted by "dengzhhu653 (via GitHub)" <gi...@apache.org>.
dengzhhu653 commented on code in PR #5950:
URL: https://github.com/apache/hive/pull/5950#discussion_r2437809045
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/Msck.java:
##########
@@ -240,6 +246,76 @@ public int repair(MsckInfo msckInfo) throws TException, MetastoreException, IOEx
}
}
+ // Generate small files warnings only for partsNotInMs
+ try {
+ // Threshold in bytes for average file size considered "small"
+ final long threshold =
+ MetastoreConf.getLongVar(getConf(), MetastoreConf.ConfVars.MSCK_SMALLFILES_AVG_SIZE);
+
+ // Collect partition names
+ final List<String> names = new ArrayList<>(partsNotInMs.size());
+ for (CheckResult.PartitionResult pr : partsNotInMs) {
+ final String name = pr.getPartitionName();
+ if (name != null && !name.isEmpty()) {
+ names.add(name);
+ }
+ }
+
+ // Batch over the names to limit single-RPC payload.
+ final int BATCH = 1000;
+ final Map<String, Partition> byName = new HashMap<>(names.size() * 2);
+
+ for (int i = 0; i < names.size(); i += BATCH) {
+ final List<String> batch = names.subList(i, Math.min(i + BATCH, names.size()));
+ final GetPartitionsByNamesRequest req = new GetPartitionsByNamesRequest(table.getDbName(), table.getTableName());
+ req.setNames(batch);
+
+ // In this branch, getPartitionsByNames returns List<Partition>
+ List<Partition> plist;
+ try {
+ @SuppressWarnings("unchecked")
+ List<Partition> tmp = (List<Partition>) getMsc().getPartitionsByNames(req);
+ plist = (tmp != null) ? tmp : Collections.emptyList();
+ } catch (NoSuchObjectException e) {
+ plist = Collections.emptyList();
+ }
+
+ for (Partition p : plist) {
+ final String pName = Warehouse.makePartName(table.getPartitionKeys(), p.getValues());
+ byName.put(pName, p);
+ }
+ }
+
+ // Build small-files stats for partitions that have quick HMS stats.
+ final Map<String, String> smallFilesStats = new TreeMap<>();
+
+ for (String pName : names) {
Review Comment:
why not `for (Partition p : plist)`
--
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-28813: MSCK/Analyze commands can show a warning in console for Small files. [hive]
Posted by "DanielZhu58 (via GitHub)" <gi...@apache.org>.
DanielZhu58 commented on code in PR #5950:
URL: https://github.com/apache/hive/pull/5950#discussion_r2444109060
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/Msck.java:
##########
@@ -240,6 +248,68 @@ public int repair(MsckInfo msckInfo) throws TException, MetastoreException, IOEx
}
}
+ // Generate small files warnings only for partsNotInMs
+ try {
+ // Threshold in bytes for average file size considered "small"
+ final long threshold =
Review Comment:
Acknowledged.
--
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-28813: MSCK/Analyze commands can show a warning in console for Small files. [hive]
Posted by "dengzhhu653 (via GitHub)" <gi...@apache.org>.
dengzhhu653 commented on code in PR #5950:
URL: https://github.com/apache/hive/pull/5950#discussion_r2430892360
##########
ql/src/java/org/apache/hadoop/hive/ql/stats/BasicStatsNoJobTask.java:
##########
@@ -181,6 +185,21 @@ public void run() {
}
parameters.putAll(basicStatistics);
StatsSetupConst.setBasicStatsState(parameters, StatsSetupConst.TRUE);
+ try {
Review Comment:
can this be simplified as
```
String who = (partish.getPartition() == null) ? ("table " + partish.getTable().getFullyQualifiedName())
: ("partition " + partish.getPartition().getName());
SmallFilesWarningUtil.showSmallFileWarnings(SessionState.get() != null ? SessionState.get().getConsole() : LOG, parameters, conf, who, "[ANALYZE][NOSCAN]");
```
--
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-28813: MSCK/Analyze commands can show a warning in console for Small files. [hive]
Posted by "DanielZhu58 (via GitHub)" <gi...@apache.org>.
DanielZhu58 commented on code in PR #5950:
URL: https://github.com/apache/hive/pull/5950#discussion_r2435294732
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/Msck.java:
##########
@@ -240,6 +244,61 @@ public int repair(MsckInfo msckInfo) throws TException, MetastoreException, IOEx
}
}
+ // Generate small files warnings only for partsNotInMs
+ try {
+ long threshold = MetastoreConf.getLongVar(getConf(), MetastoreConf.ConfVars.MSCK_SMALLFILES_AVG_SIZE);
+ Map<String, String> smallFilesStats = new TreeMap<>();
+ for (CheckResult.PartitionResult pr : partsNotInMs) {
+ List<String> statsVals;
+ try {
+ // parse the partition name into a few partition values
+ statsVals = Warehouse.makeValsFromName(pr.getPartitionName(), null);
+ } catch (MetaException me) {
+ // skip this partition if this somehow failed
+ continue;
+ }
+ Partition p;
+ try {
+ // the newly added partition should exist in metastore
+ p = getMsc().getPartition(table.getCatName(), table.getDbName(), table.getTableName(), statsVals);
+ } catch (NoSuchObjectException nsoe) {
+ // the partition is not actually added in metastore
+ continue;
+ }
+ Map<String, String> params = p.getParameters();
Review Comment:
Acknowledged. The code is changed.
--
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-28813: MSCK/Analyze commands can show a warning in console for Small files. [hive]
Posted by "DanielZhu58 (via GitHub)" <gi...@apache.org>.
DanielZhu58 commented on code in PR #5950:
URL: https://github.com/apache/hive/pull/5950#discussion_r2369242569
##########
ql/src/java/org/apache/hadoop/hive/ql/exec/StatsTask.java:
##########
@@ -124,6 +133,125 @@ public int execute() {
return 0;
}
+ /**
+ * Check for small files after ANALYZE completes
+ */
+ private void checkSmallFilesAfterAnalyze(Hive db, Table tbl) {
+ // Use the existing merge small files threshold configuration
+ long smallFilesThreshold = conf.getLongVar(HiveConf.ConfVars.HIVE_MERGE_MAP_FILES_AVG_SIZE);
+ List<String> smallFilesWarnings = new ArrayList<>();
+
+ // Get the console from SessionState for output
+ PrintStream console = SessionState.get().out;
Review Comment:
This whole code part is removed already. Thanks.
--
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-28813: MSCK/Analyze commands can show a warning in console for Small files. [hive]
Posted by "DanielZhu58 (via GitHub)" <gi...@apache.org>.
DanielZhu58 commented on code in PR #5950:
URL: https://github.com/apache/hive/pull/5950#discussion_r2381290876
##########
ql/src/java/org/apache/hadoop/hive/ql/exec/StatsTask.java:
##########
@@ -124,7 +124,6 @@ public int execute() {
return 0;
}
-
Review Comment:
Acknowledged.
--
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-28813: MSCK/Analyze commands can show a warning in console for Small files. [hive]
Posted by "DanielZhu58 (via GitHub)" <gi...@apache.org>.
DanielZhu58 commented on code in PR #5950:
URL: https://github.com/apache/hive/pull/5950#discussion_r2381312436
##########
standalone-metastore/metastore-server/pom.xml:
##########
@@ -470,6 +470,12 @@
<artifactId>mssqlserver</artifactId>
<scope>test</scope>
</dependency>
+ <dependency>
+ <groupId>org.apache.hive</groupId>
Review Comment:
To retrieve the HiveConf.ConfVars.HIVE_MERGE_MAP_FILES_AVG_SIZE.
--
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-28813: MSCK/Analyze commands can show a warning in console for Small files. [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #5950:
URL: https://github.com/apache/hive/pull/5950#discussion_r2201495706
##########
ql/src/java/org/apache/hadoop/hive/ql/ddl/misc/msck/MsckOperation.java:
##########
@@ -75,7 +78,18 @@ public int execute() throws HiveException, IOException, TException, MetastoreExc
MsckInfo msckInfo = new MsckInfo(SessionState.get().getCurrentCatalog(), tableName.getDb(), tableName.getTable(),
desc.getFilterExp(), desc.getResFile(), desc.isRepairPartitions(),
desc.isAddPartitions(), desc.isDropPartitions(), partitionExpirySeconds);
- return msck.repair(msckInfo);
+ int result = msck.repair(msckInfo);
+ if (msckInfo.getSmallFilesStats() != null && !msckInfo.getSmallFilesStats().isEmpty()){
Review Comment:
can we move the log the warning inside of `msck.repair` function?
##########
ql/src/java/org/apache/hadoop/hive/ql/ddl/misc/msck/MsckOperation.java:
##########
@@ -75,7 +78,18 @@ public int execute() throws HiveException, IOException, TException, MetastoreExc
MsckInfo msckInfo = new MsckInfo(SessionState.get().getCurrentCatalog(), tableName.getDb(), tableName.getTable(),
desc.getFilterExp(), desc.getResFile(), desc.isRepairPartitions(),
desc.isAddPartitions(), desc.isDropPartitions(), partitionExpirySeconds);
- return msck.repair(msckInfo);
+ int result = msck.repair(msckInfo);
+ if (msckInfo.getSmallFilesStats() != null && !msckInfo.getSmallFilesStats().isEmpty()){
Review Comment:
can we move the log warning inside of `msck.repair` function?
--
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-28813: MSCK/Analyze commands can show a warning in console for Small files. [hive]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #5950:
URL: https://github.com/apache/hive/pull/5950#issuecomment-3405600673
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5950) **Quality Gate passed**
Issues
 [55 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5950&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5950&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=5950&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5950&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5950&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5950)
--
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-28813: MSCK/Analyze commands can show a warning in console for Small files. [hive]
Posted by "dengzhhu653 (via GitHub)" <gi...@apache.org>.
dengzhhu653 commented on code in PR #5950:
URL: https://github.com/apache/hive/pull/5950#discussion_r2430919630
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/Msck.java:
##########
@@ -240,6 +244,61 @@ public int repair(MsckInfo msckInfo) throws TException, MetastoreException, IOEx
}
}
+ // Generate small files warnings only for partsNotInMs
+ try {
+ long threshold = MetastoreConf.getLongVar(getConf(), MetastoreConf.ConfVars.MSCK_SMALLFILES_AVG_SIZE);
+ Map<String, String> smallFilesStats = new TreeMap<>();
+ for (CheckResult.PartitionResult pr : partsNotInMs) {
+ List<String> statsVals;
+ try {
+ // parse the partition name into a few partition values
+ statsVals = Warehouse.makeValsFromName(pr.getPartitionName(), null);
+ } catch (MetaException me) {
+ // skip this partition if this somehow failed
+ continue;
+ }
+ Partition p;
+ try {
+ // the newly added partition should exist in metastore
+ p = getMsc().getPartition(table.getCatName(), table.getDbName(), table.getTableName(), statsVals);
+ } catch (NoSuchObjectException nsoe) {
+ // the partition is not actually added in metastore
+ continue;
+ }
+ Map<String, String> params = p.getParameters();
Review Comment:
can we add a new method `static boolean smallAverageFilesDetected(getConf(), params)` in `SmallFilesWarningUtil` to include the small average file validation?
--
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-28813: MSCK/Analyze commands can show a warning in console for Small files. [hive]
Posted by "DanielZhu58 (via GitHub)" <gi...@apache.org>.
DanielZhu58 commented on code in PR #5950:
URL: https://github.com/apache/hive/pull/5950#discussion_r2443971795
##########
ql/src/java/org/apache/hadoop/hive/ql/stats/BasicStatsNoJobTask.java:
##########
@@ -293,6 +307,15 @@ public void run() {
parameters.put(StatsSetupConst.NUM_FILES, String.valueOf(numFiles));
parameters.put(StatsSetupConst.NUM_ERASURE_CODED_FILES, String.valueOf(numErasureCodedFiles));
+ try {
+ String who = (partish.getPartition() == null) ? ("table " + partish.getTable().getFullyQualifiedName())
+ : ("partition " + partish.getPartition().getName());
+ long threshold = conf.getLongVar(HiveConf.ConfVars.HIVE_MERGE_MAP_FILES_AVG_SIZE);
+ SmallFilesWarningUtil.smallFilesWarnings(parameters, 100L, threshold, who, "[ANALYZE][NOSCAN]")
+ .ifPresent(smallFilesConsoleAndWarn(LOG));
+ } catch (Throwable t) {
Review Comment:
Acknowledged.
--
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-28813: MSCK/Analyze commands can show a warning in console for Small files. [hive]
Posted by "dengzhhu653 (via GitHub)" <gi...@apache.org>.
dengzhhu653 commented on code in PR #5950:
URL: https://github.com/apache/hive/pull/5950#discussion_r2444081176
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/Msck.java:
##########
@@ -26,24 +26,31 @@
import java.util.AbstractList;
import java.util.ArrayList;
import java.util.Collections;
+import java.util.HashMap;
import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
+import java.util.Objects;
import java.util.Set;
+import java.util.TreeMap;
Review Comment:
nit: unused import
--
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-28813: MSCK/Analyze commands can show a warning in console for Small files. [hive]
Posted by "DanielZhu58 (via GitHub)" <gi...@apache.org>.
DanielZhu58 commented on code in PR #5950:
URL: https://github.com/apache/hive/pull/5950#discussion_r2435163981
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/Msck.java:
##########
@@ -240,6 +244,61 @@ public int repair(MsckInfo msckInfo) throws TException, MetastoreException, IOEx
}
}
+ // Generate small files warnings only for partsNotInMs
+ try {
+ long threshold = MetastoreConf.getLongVar(getConf(), MetastoreConf.ConfVars.MSCK_SMALLFILES_AVG_SIZE);
+ Map<String, String> smallFilesStats = new TreeMap<>();
+ for (CheckResult.PartitionResult pr : partsNotInMs) {
+ List<String> statsVals;
+ try {
+ // parse the partition name into a few partition values
+ statsVals = Warehouse.makeValsFromName(pr.getPartitionName(), null);
+ } catch (MetaException me) {
+ // skip this partition if this somehow failed
+ continue;
+ }
+ Partition p;
+ try {
+ // the newly added partition should exist in metastore
+ p = getMsc().getPartition(table.getCatName(), table.getDbName(), table.getTableName(), statsVals);
Review Comment:
Acknowledged. I changed the code about get partitions.
--
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-28813: MSCK/Analyze commands can show a warning in console for Small files. [hive]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #5950:
URL: https://github.com/apache/hive/pull/5950#issuecomment-3410456906
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5950) **Quality Gate passed**
Issues
 [148 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5950&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5950&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=5950&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5950&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5950&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5950)
--
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-28813: MSCK/Analyze commands can show a warning in console for Small files. [hive]
Posted by "dengzhhu653 (via GitHub)" <gi...@apache.org>.
dengzhhu653 commented on code in PR #5950:
URL: https://github.com/apache/hive/pull/5950#discussion_r2348115212
##########
ql/src/java/org/apache/hadoop/hive/ql/exec/StatsTask.java:
##########
@@ -124,6 +133,125 @@ public int execute() {
return 0;
}
+ /**
+ * Check for small files after ANALYZE completes
+ */
+ private void checkSmallFilesAfterAnalyze(Hive db, Table tbl) {
+ // Use the existing merge small files threshold configuration
+ long smallFilesThreshold = conf.getLongVar(HiveConf.ConfVars.HIVE_MERGE_MAP_FILES_AVG_SIZE);
+ List<String> smallFilesWarnings = new ArrayList<>();
+
+ // Get the console from SessionState for output
+ PrintStream console = SessionState.get().out;
Review Comment:
we might need to check whether `SessionState.get()` is null or not
--
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-28813: MSCK/Analyze commands can show a warning in console for Small files. [hive]
Posted by "DanielZhu58 (via GitHub)" <gi...@apache.org>.
DanielZhu58 commented on code in PR #5950:
URL: https://github.com/apache/hive/pull/5950#discussion_r2381324534
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreChecker.java:
##########
@@ -358,6 +361,28 @@ void checkTable(Table table, PartitionIterable parts, byte[] filterExp, CheckRes
if (partPath == null) {
continue;
}
+
+ // check the table/partition if (totalSize / numFiles) is less than avgFileSize
+ Map<String, String> partitionParameters = partition.getParameters();
+ if (partitionParameters != null) {
+ long totalSize = Long.parseLong(partitionParameters.getOrDefault(StatsSetupConst.TOTAL_SIZE, "0"));
Review Comment:
Acknowledged. The case will be handled in the newly added partitions instead of here.
--
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-28813: MSCK/Analyze commands can show a warning in console for Small files. [hive]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #5950:
URL: https://github.com/apache/hive/pull/5950#issuecomment-3337906741
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5950) **Quality Gate passed**
Issues
 [30 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5950&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5950&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=5950&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5950&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5950&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5950)
--
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-28813: MSCK/Analyze commands can show a warning in console for Small files. [hive]
Posted by "dengzhhu653 (via GitHub)" <gi...@apache.org>.
dengzhhu653 commented on code in PR #5950:
URL: https://github.com/apache/hive/pull/5950#discussion_r2437813745
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/Msck.java:
##########
@@ -240,6 +246,76 @@ public int repair(MsckInfo msckInfo) throws TException, MetastoreException, IOEx
}
}
+ // Generate small files warnings only for partsNotInMs
+ try {
+ // Threshold in bytes for average file size considered "small"
+ final long threshold =
+ MetastoreConf.getLongVar(getConf(), MetastoreConf.ConfVars.MSCK_SMALLFILES_AVG_SIZE);
+
+ // Collect partition names
+ final List<String> names = new ArrayList<>(partsNotInMs.size());
+ for (CheckResult.PartitionResult pr : partsNotInMs) {
+ final String name = pr.getPartitionName();
+ if (name != null && !name.isEmpty()) {
+ names.add(name);
+ }
+ }
+
+ // Batch over the names to limit single-RPC payload.
+ final int BATCH = 1000;
+ final Map<String, Partition> byName = new HashMap<>(names.size() * 2);
+
+ for (int i = 0; i < names.size(); i += BATCH) {
+ final List<String> batch = names.subList(i, Math.min(i + BATCH, names.size()));
+ final GetPartitionsByNamesRequest req = new GetPartitionsByNamesRequest(table.getDbName(), table.getTableName());
+ req.setNames(batch);
+
+ // In this branch, getPartitionsByNames returns List<Partition>
+ List<Partition> plist;
+ try {
+ @SuppressWarnings("unchecked")
+ List<Partition> tmp = (List<Partition>) getMsc().getPartitionsByNames(req);
+ plist = (tmp != null) ? tmp : Collections.emptyList();
+ } catch (NoSuchObjectException e) {
+ plist = Collections.emptyList();
+ }
+
+ for (Partition p : plist) {
+ final String pName = Warehouse.makePartName(table.getPartitionKeys(), p.getValues());
+ byName.put(pName, p);
+ }
+ }
+
+ // Build small-files stats for partitions that have quick HMS stats.
+ final Map<String, String> smallFilesStats = new TreeMap<>();
+
+ for (String pName : names) {
+ final Partition p = byName.get(pName);
+ if (p == null) {
+ // not found / not visible yet
+ continue;
+ }
+
+ final Map<String, String> params = p.getParameters();
+ // Use util to decide if this partition should trigger the warning
+ if (SmallFilesWarningUtil.smallAverageFilesDetected(SmallFilesWarningUtil.DEFAULT_MIN_FILES, threshold, params)) {
+ // We know stats exist and are well-formed; compute numbers for the value string
+ final long totalSize = Long.parseLong(params.get(StatsSetupConst.TOTAL_SIZE));
+ final long numFiles = Long.parseLong(params.get(StatsSetupConst.NUM_FILES));
+ final long avg = Math.floorDiv(totalSize, numFiles);
+ smallFilesStats.put(pName, "avgBytes=" + avg + ", partition total files=" + numFiles + ", totalBytes=" + totalSize);
+ }
+ }
+
+ msckInfo.setSmallFilesStats(smallFilesStats.isEmpty() ? Collections.emptyMap() : smallFilesStats);
Review Comment:
why not `msckInfo.setSmallFilesStat(smallFilesStats)`
--
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-28813: MSCK/Analyze commands can show a warning in console for Small files. [hive]
Posted by "DanielZhu58 (via GitHub)" <gi...@apache.org>.
DanielZhu58 commented on code in PR #5950:
URL: https://github.com/apache/hive/pull/5950#discussion_r2443568460
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/Msck.java:
##########
@@ -240,6 +246,76 @@ public int repair(MsckInfo msckInfo) throws TException, MetastoreException, IOEx
}
}
+ // Generate small files warnings only for partsNotInMs
+ try {
+ // Threshold in bytes for average file size considered "small"
+ final long threshold =
+ MetastoreConf.getLongVar(getConf(), MetastoreConf.ConfVars.MSCK_SMALLFILES_AVG_SIZE);
+
+ // Collect partition names
+ final List<String> names = new ArrayList<>(partsNotInMs.size());
+ for (CheckResult.PartitionResult pr : partsNotInMs) {
Review Comment:
Acknowledged. Now using this:
final List<String> names = partsNotInMs.stream()
.map(CheckResult.PartitionResult::getPartitionName)
.filter(Objects::nonNull)
.filter(s -> !s.isEmpty())
.collect(Collectors.toList());
--
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-28813: MSCK/Analyze commands can show a warning in console for Small files. [hive]
Posted by "DanielZhu58 (via GitHub)" <gi...@apache.org>.
DanielZhu58 commented on code in PR #5950:
URL: https://github.com/apache/hive/pull/5950#discussion_r2443872727
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/Msck.java:
##########
@@ -240,6 +248,74 @@ public int repair(MsckInfo msckInfo) throws TException, MetastoreException, IOEx
}
}
+ // Generate small files warnings only for partsNotInMs
+ try {
+ // Threshold in bytes for average file size considered "small"
+ final long threshold =
+ MetastoreConf.getLongVar(getConf(), MetastoreConf.ConfVars.MSCK_SMALLFILES_AVG_SIZE);
+
+ // Collect partition names
+ final List<String> names = partsNotInMs.stream()
+ .map(CheckResult.PartitionResult::getPartitionName)
+ .filter(Objects::nonNull)
+ .filter(s -> !s.isEmpty())
+ .collect(Collectors.toList());
+
+ // Batch over the names to limit single-RPC payload.
+ final int PART_FETCH_BATCH = 1000;
+ final Map<String, Partition> byName = new HashMap<>(names.size() * 2);
+ List<Partition> allParts = Batchable.runBatched(
+ PART_FETCH_BATCH,
+ names,
+ new Batchable<String, Partition>() {
+ @Override
+ public List<Partition> run(List<String> batch) throws Exception {
+ final GetPartitionsByNamesRequest req =
+ new GetPartitionsByNamesRequest(table.getDbName(), table.getTableName());
+ req.setNames(batch);
+ try {
+ @SuppressWarnings("unchecked")
+ List<Partition> plist = (List<Partition>) getMsc().getPartitionsByNames(req);
+ return (plist != null) ? plist : Collections.emptyList();
+ } catch (NoSuchObjectException e) {
+ return Collections.emptyList();
+ }
+ }
+ });
+
+ for (Partition p : allParts) {
+ final String pName = Warehouse.makePartName(table.getPartitionKeys(), p.getValues());
+ byName.put(pName, p);
+ }
+
+ // Build small-files stats for partitions that have quick HMS stats.
+ final Map<String, String> smallFilesStats = new TreeMap<>();
+
+ for (String pName : names) {
+ final Partition p = byName.get(pName);
+ if (p == null) {
+ // not found / not visible yet
+ continue;
+ }
+
+ final Map<String, String> params = p.getParameters();
+ // Use util to decide if this partition should trigger the warning
+ if (SmallFilesWarningUtil.smallAverageFilesDetected(SmallFilesWarningUtil.DEFAULT_MIN_FILES, threshold, params)) {
Review Comment:
Acknowledged.
--
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-28813: MSCK/Analyze commands can show a warning in console for Small files. [hive]
Posted by "DanielZhu58 (via GitHub)" <gi...@apache.org>.
DanielZhu58 commented on code in PR #5950:
URL: https://github.com/apache/hive/pull/5950#discussion_r2443868205
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/Msck.java:
##########
@@ -240,6 +248,74 @@ public int repair(MsckInfo msckInfo) throws TException, MetastoreException, IOEx
}
}
+ // Generate small files warnings only for partsNotInMs
+ try {
+ // Threshold in bytes for average file size considered "small"
+ final long threshold =
+ MetastoreConf.getLongVar(getConf(), MetastoreConf.ConfVars.MSCK_SMALLFILES_AVG_SIZE);
+
+ // Collect partition names
+ final List<String> names = partsNotInMs.stream()
+ .map(CheckResult.PartitionResult::getPartitionName)
+ .filter(Objects::nonNull)
+ .filter(s -> !s.isEmpty())
+ .collect(Collectors.toList());
+
+ // Batch over the names to limit single-RPC payload.
+ final int PART_FETCH_BATCH = 1000;
+ final Map<String, Partition> byName = new HashMap<>(names.size() * 2);
+ List<Partition> allParts = Batchable.runBatched(
+ PART_FETCH_BATCH,
+ names,
+ new Batchable<String, Partition>() {
+ @Override
+ public List<Partition> run(List<String> batch) throws Exception {
+ final GetPartitionsByNamesRequest req =
+ new GetPartitionsByNamesRequest(table.getDbName(), table.getTableName());
+ req.setNames(batch);
+ try {
+ @SuppressWarnings("unchecked")
+ List<Partition> plist = (List<Partition>) getMsc().getPartitionsByNames(req);
+ return (plist != null) ? plist : Collections.emptyList();
+ } catch (NoSuchObjectException e) {
+ return Collections.emptyList();
+ }
+ }
+ });
+
+ for (Partition p : allParts) {
+ final String pName = Warehouse.makePartName(table.getPartitionKeys(), p.getValues());
+ byName.put(pName, p);
+ }
+
+ // Build small-files stats for partitions that have quick HMS stats.
+ final Map<String, String> smallFilesStats = new TreeMap<>();
+
+ for (String pName : names) {
+ final Partition p = byName.get(pName);
+ if (p == null) {
Review Comment:
Acknowledged.
--
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-28813: MSCK/Analyze commands can show a warning in console for Small files. [hive]
Posted by "DanielZhu58 (via GitHub)" <gi...@apache.org>.
DanielZhu58 commented on code in PR #5950:
URL: https://github.com/apache/hive/pull/5950#discussion_r2443962675
##########
ql/src/java/org/apache/hadoop/hive/ql/stats/BasicStatsNoJobTask.java:
##########
@@ -307,7 +330,16 @@ public void run() {
console.printInfo("[Warning] could not update stats for " + partish.getSimpleName() + ".", "Failed with exception " + e.getMessage() + "\n" + StringUtils.stringifyException(e));
}
}
+ }
+ public static Consumer<String> smallFilesConsoleAndWarn(Logger log) {
Review Comment:
Acknowledged.
--
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-28813: MSCK/Analyze commands can show a warning in console for Small files. [hive]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #5950:
URL: https://github.com/apache/hive/pull/5950#issuecomment-3421085327
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5950) **Quality Gate passed**
Issues
 [66 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5950&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5950&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=5950&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5950&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5950&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5950)
--
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-28813: MSCK/Analyze commands can show a warning in console for Small files. [hive]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #5950:
URL: https://github.com/apache/hive/pull/5950#issuecomment-3425937709
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5950) **Quality Gate passed**
Issues
 [61 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5950&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5950&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=5950&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5950&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5950&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5950)
--
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-28813: MSCK/Analyze commands can show a warning in console for Small files. [hive]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #5950:
URL: https://github.com/apache/hive/pull/5950#issuecomment-3312649677
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5950) **Quality Gate passed**
Issues
 [38 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5950&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5950&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=5950&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5950&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5950&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5950)
--
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-28813: MSCK/Analyze commands can show a warning in console for Small files. [hive]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #5950:
URL: https://github.com/apache/hive/pull/5950#issuecomment-3365512112
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5950) **Quality Gate passed**
Issues
 [32 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5950&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5950&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=5950&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5950&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5950&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5950)
--
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-28813: MSCK/Analyze commands can show a warning in console for Small files. [hive]
Posted by "dengzhhu653 (via GitHub)" <gi...@apache.org>.
dengzhhu653 commented on code in PR #5950:
URL: https://github.com/apache/hive/pull/5950#discussion_r2370987566
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreChecker.java:
##########
@@ -358,6 +361,28 @@ void checkTable(Table table, PartitionIterable parts, byte[] filterExp, CheckRes
if (partPath == null) {
continue;
}
+
+ // check the table/partition if (totalSize / numFiles) is less than avgFileSize
+ Map<String, String> partitionParameters = partition.getParameters();
+ if (partitionParameters != null) {
+ long totalSize = Long.parseLong(partitionParameters.getOrDefault(StatsSetupConst.TOTAL_SIZE, "0"));
Review Comment:
if we can take care of the newly added partition, then can this removed?
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreChecker.java:
##########
@@ -358,6 +361,28 @@ void checkTable(Table table, PartitionIterable parts, byte[] filterExp, CheckRes
if (partPath == null) {
continue;
}
+
+ // check the table/partition if (totalSize / numFiles) is less than avgFileSize
+ Map<String, String> partitionParameters = partition.getParameters();
+ if (partitionParameters != null) {
+ long totalSize = Long.parseLong(partitionParameters.getOrDefault(StatsSetupConst.TOTAL_SIZE, "0"));
Review Comment:
if we can take care of the newly added partition, then can this be removed?
--
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-28813: MSCK/Analyze commands can show a warning in console for Small files. [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #5950:
URL: https://github.com/apache/hive/pull/5950#discussion_r2293469689
##########
ql/src/java/org/apache/hadoop/hive/ql/ddl/misc/msck/MsckOperation.java:
##########
@@ -75,7 +78,25 @@ public int execute() throws HiveException, IOException, TException, MetastoreExc
MsckInfo msckInfo = new MsckInfo(SessionState.get().getCurrentCatalog(), tableName.getDb(), tableName.getTable(),
desc.getFilterExp(), desc.getResFile(), desc.isRepairPartitions(),
desc.isAddPartitions(), desc.isDropPartitions(), partitionExpirySeconds);
- return msck.repair(msckInfo);
+ int result = msck.repair(msckInfo);
+ if (msckInfo.getSmallFilesStats() != null && !msckInfo.getSmallFilesStats().isEmpty()){
+ List<String> logInfo = new ArrayList<>(msckInfo.getSmallFilesStats().size());
+ for (Map.Entry<String, String> me : msckInfo.getSmallFilesStats().entrySet()){
+ String partitionName = me.getKey();
+ String smallFilesStats = me.getValue();
+ logInfo.add("This table/partition average file size is less than Hive average file size.\n " +
Review Comment:
should this be under warn?
--
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-28813: MSCK/Analyze commands can show a warning in console for Small files. [hive]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #5950:
URL: https://github.com/apache/hive/pull/5950#issuecomment-3401227859
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5950) **Quality Gate passed**
Issues
 [55 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5950&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5950&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=5950&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5950&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5950&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5950)
--
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-28813: MSCK/Analyze commands can show a warning in console for Small files. [hive]
Posted by "DanielZhu58 (via GitHub)" <gi...@apache.org>.
DanielZhu58 commented on code in PR #5950:
URL: https://github.com/apache/hive/pull/5950#discussion_r2443955588
##########
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/utils/SmallFilesWarningUtil.java:
##########
@@ -0,0 +1,104 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hive.metastore.utils;
+
+import java.util.Map;
+import java.util.Optional;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hive.common.StatsSetupConst;
+import org.apache.hadoop.hive.metastore.conf.MetastoreConf;
+
+public final class SmallFilesWarningUtil {
+ private SmallFilesWarningUtil() {}
+
+ /** Default minimum number of files before we consider emitting the warning. */
+ public static final long DEFAULT_MIN_FILES = 100L;
+
+ /**
+ * True if quick HMS stats in {@code parameters} indicate small average file size.
+ * Missing/malformed stats or insufficient file count return false.
+ */
+ public static boolean smallAverageFilesDetected(long minFiles,
+ long avgSizeThreshold,
+ Map<String, String> parameters) {
+ if (parameters == null) return false;
+
+ final String ts = parameters.get(StatsSetupConst.TOTAL_SIZE);
+ final String nf = parameters.get(StatsSetupConst.NUM_FILES);
+ if (ts == null || nf == null) return false;
+
+ final long totalSize;
+ final long numFiles;
+ try {
+ totalSize = Long.parseLong(ts);
+ numFiles = Long.parseLong(nf);
+ } catch (NumberFormatException ignore) {
+ return false;
+ }
+
+ if (numFiles <= minFiles || totalSize <= 0) return false;
+
+ final long avg = Math.floorDiv(totalSize, numFiles);
+ return avg <= avgSizeThreshold;
+ }
+
+ /**
+ * Convenience overload: reads the threshold from {@code conf} and uses {@link #DEFAULT_MIN_FILES}.
+ * Returns false if {@code conf} is null.
+ */
+ public static boolean smallAverageFilesDetected(Configuration conf,
+ Map<String, String> parameters) {
+ if (conf == null) return false;
Review Comment:
Normally it would not be null.
--
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-28813: MSCK/Analyze commands can show a warning in console for Small files. [hive]
Posted by "DanielZhu58 (via GitHub)" <gi...@apache.org>.
DanielZhu58 commented on code in PR #5950:
URL: https://github.com/apache/hive/pull/5950#discussion_r2444118807
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/Msck.java:
##########
@@ -26,24 +26,31 @@
import java.util.AbstractList;
import java.util.ArrayList;
import java.util.Collections;
+import java.util.HashMap;
import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
+import java.util.Objects;
import java.util.Set;
+import java.util.TreeMap;
Review Comment:
They are used around line 256, line 287.
--
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-28813: MSCK/Analyze commands can show a warning in console for Small files. [hive]
Posted by "dengzhhu653 (via GitHub)" <gi...@apache.org>.
dengzhhu653 commented on code in PR #5950:
URL: https://github.com/apache/hive/pull/5950#discussion_r2443658359
##########
ql/src/java/org/apache/hadoop/hive/ql/stats/BasicStatsNoJobTask.java:
##########
@@ -307,7 +330,16 @@ public void run() {
console.printInfo("[Warning] could not update stats for " + partish.getSimpleName() + ".", "Failed with exception " + e.getMessage() + "\n" + StringUtils.stringifyException(e));
}
}
+ }
+ public static Consumer<String> smallFilesConsoleAndWarn(Logger log) {
Review Comment:
if this method is just for a simple message, we can just move it to the caller, e.g,
```
.ifPresent(s -> {
SessionState ss = SessionState.get();
if (ss != null && ss.getConsole() != null) {
ss.getConsole().printInfo(msg);
} else {
LOG.warn(msg);
}
}
```
--
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-28813: MSCK/Analyze commands can show a warning in console for Small files. [hive]
Posted by "dengzhhu653 (via GitHub)" <gi...@apache.org>.
dengzhhu653 commented on code in PR #5950:
URL: https://github.com/apache/hive/pull/5950#discussion_r2443658359
##########
ql/src/java/org/apache/hadoop/hive/ql/stats/BasicStatsNoJobTask.java:
##########
@@ -307,7 +330,16 @@ public void run() {
console.printInfo("[Warning] could not update stats for " + partish.getSimpleName() + ".", "Failed with exception " + e.getMessage() + "\n" + StringUtils.stringifyException(e));
}
}
+ }
+ public static Consumer<String> smallFilesConsoleAndWarn(Logger log) {
Review Comment:
if this method is just for a simple message, we can just move it to the caller, e.g,
```
.ifPresent(s -> {
SessionState ss = SessionState.get();
if (ss != null && ss.getConsole() != null) {
ss.getConsole().printInfo(msg);
} else {
log.warn(msg);
}
}
```
--
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-28813: MSCK/Analyze commands can show a warning in console for Small files. [hive]
Posted by "DanielZhu58 (via GitHub)" <gi...@apache.org>.
DanielZhu58 commented on code in PR #5950:
URL: https://github.com/apache/hive/pull/5950#discussion_r2443587638
##########
ql/src/java/org/apache/hadoop/hive/ql/stats/BasicStatsNoJobTask.java:
##########
@@ -181,6 +185,21 @@ public void run() {
}
parameters.putAll(basicStatistics);
StatsSetupConst.setBasicStatsState(parameters, StatsSetupConst.TRUE);
+ try {
+ String who = (partish.getPartition() == null) ? ("table " + partish.getTable().getFullyQualifiedName())
+ : ("partition " + partish.getPartition().getName());
+ long threshold = conf.getLongVar(HiveConf.ConfVars.HIVE_MERGE_MAP_FILES_AVG_SIZE);
+ SmallFilesWarningUtil.smallFilesWarnings(parameters, 100L, threshold, who, "[ANALYZE][NOSCAN]")
+ .ifPresent(msg -> {
+ LOG.info(msg);
+ SessionState ss = SessionState.get();
+ if (ss != null && ss.getConsole() != null) {
+ ss.getConsole().printInfo(msg);
+ }
+ });
+ } catch (Throwable t) {
Review Comment:
Actually there is no chance that `SmallFilesWarningUtil.smallFilesWarnings` will throw exception.
The util does no I/O and no RPC—it just reads a Map, parses two longs, and formats a string. And all the possible failures haven been considered.
params == null → returns Optional.empty()
missing keys → returns Optional.empty()
bad numbers → caught as NumberFormatException and returns Optional.empty()
--
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-28813: MSCK/Analyze commands can show a warning in console for Small files. [hive]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #5950:
URL: https://github.com/apache/hive/pull/5950#issuecomment-3424780052
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5950) **Quality Gate passed**
Issues
 [61 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5950&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5950&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=5950&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5950&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5950&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5950)
--
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-28813: MSCK/Analyze commands can show a warning in console for Small files. [hive]
Posted by "DanielZhu58 (via GitHub)" <gi...@apache.org>.
DanielZhu58 commented on code in PR #5950:
URL: https://github.com/apache/hive/pull/5950#discussion_r2458536726
##########
standalone-metastore/metastore-server/pom.xml:
##########
@@ -470,6 +470,12 @@
<artifactId>mssqlserver</artifactId>
<scope>test</scope>
</dependency>
+ <dependency>
+ <groupId>org.apache.hive</groupId>
Review Comment:
This dependency is now removed.
--
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-28813: MSCK/Analyze commands can show a warning in console for Small files. [hive]
Posted by "dengzhhu653 (via GitHub)" <gi...@apache.org>.
dengzhhu653 commented on code in PR #5950:
URL: https://github.com/apache/hive/pull/5950#discussion_r2371014538
##########
ql/src/java/org/apache/hadoop/hive/ql/exec/StatsTask.java:
##########
@@ -124,7 +124,6 @@ public int execute() {
return 0;
}
-
Review Comment:
nit: remove this change
--
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-28813: MSCK/Analyze commands can show a warning in console for Small files. [hive]
Posted by "dengzhhu653 (via GitHub)" <gi...@apache.org>.
dengzhhu653 commented on code in PR #5950:
URL: https://github.com/apache/hive/pull/5950#discussion_r2371013108
##########
ql/src/java/org/apache/hadoop/hive/ql/ddl/misc/msck/MsckOperation.java:
##########
@@ -75,7 +78,25 @@ public int execute() throws HiveException, IOException, TException, MetastoreExc
MsckInfo msckInfo = new MsckInfo(SessionState.get().getCurrentCatalog(), tableName.getDb(), tableName.getTable(),
desc.getFilterExp(), desc.getResFile(), desc.isRepairPartitions(),
desc.isAddPartitions(), desc.isDropPartitions(), partitionExpirySeconds);
- return msck.repair(msckInfo);
+ int result = msck.repair(msckInfo);
+ Map<String, String> smallFilesStats = msckInfo.getSmallFilesStats();
+ if (smallFilesStats != null && !smallFilesStats.isEmpty()) {
+ // keep the small files information in the logs
+ List<String> logInfo = smallFilesStats.entrySet().stream()
+ .map(entry -> String.format(
+ "This table/partition average file size is less than Hive average file size.%n The partition name is %s. %s",
+ entry.getKey(), entry.getValue()))
+ .collect(Collectors.toList());
+ LOG.info("There are small files exist.\n{}", String.join("\n", logInfo));
Review Comment:
we just need to keep one log by `ss.getConsole().printInfo`
--
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-28813: MSCK/Analyze commands can show a warning in console for Small files. [hive]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #5950:
URL: https://github.com/apache/hive/pull/5950#issuecomment-3442136153
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5950) **Quality Gate passed**
Issues
 [63 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5950&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5950&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=5950&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5950&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5950&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5950)
--
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