Utilities using jcommander for boolean argument can be misleading

[xushiyan]

When people use Hudi utilities, many of them are not aware of --a-boolean-flag being present means true, and by following other args' pattern, they tend to set --a-boolean-flag true which resulted in

Exception in thread "main" org.apache.hudi.com.beust.jcommander.ParameterException: Was passed main parameter 'true' but no main parameter was defined in your arg class

To improve this experience, we should allow both ways to set a boolean arg. People should be able to

• omit or present a bool flag, omitting means using default value (can be true or false), present means true
• setting a true or false explicitly overwrites the default

To go further, any boolean arg used by utilities should be changed to default false. This may lead to breaking some usage, but it's a necessary step to align the expectations.

[danny0405]

+1, I got a lot of feedback because of these confusions. A explicit param true or false is more straight-forward.

[vinothchandar]

Can we list out all such boolean flags? are they all fixable without causing any side-effects 'silently' i.e changing behavior for the Hudi streamer run ..

This discussion will be more actionable with that analysis

[wombatu-kun]

Here is the list of configs in hudi-utilities that default to true (6 in total):

HoodieClusteringJob.java
    public Boolean skipClean = true;
HoodieCompactionAdminTool.java
    public boolean printOutput = true;
HoodieCompactor.java
    public Boolean skipClean = true;
HoodieDropPartitionsTool.java
    public boolean hiveUseJdbc = true;
HoodieMetadataTableValidator.java
   public boolean validateRecordIndexCount = true;
   public boolean validateRecordIndexContent = true;

And these configs default to false (62 in total):

HoodieClusteringJob.java
    public Boolean runSchedule = false;
    public Boolean retryLastFailedClusteringJob = false;
    public Boolean help = false;
HoodieCompactionAdminTool.java
    public boolean dryRun = false;
    public boolean skipValidation = false;
    public Boolean help = false;
HoodieCompactor.java
    public Boolean runSchedule = false;
    public Boolean help = false;
HoodieDataTableValidator.java
    public boolean continuous = false;
    public boolean ignoreFailed = false;
    public Boolean assumeDatePartitioning = false;
    public Boolean help = false;
HoodieDropPartitionsTool.java
    public boolean syncToHive = false;
    public boolean hiveSyncIgnoreException = false;
    public Boolean help = false;
HoodieIndexer.java
    public Boolean help = false;
HoodieMetadataTableValidator.java
    public boolean continuous = false;
    public boolean skipDataFilesForCleaning = false;
    public boolean validateLatestFileSlices = false;
    public boolean validateLatestBaseFiles = false;
    public boolean validateAllFileGroups = false;
    public boolean validateAllColumnStats = false;
    public boolean validatePartitionStats = false;
    public boolean validateBloomFilters = false;
    public boolean validateSecondaryIndex = false;
    public boolean ignoreFailed = false;
    public Boolean assumeDatePartitioning = false;
    public Boolean help = false;
HoodieRepairTool.java
    public Boolean assumeDatePartitioning = false;
    public Boolean help = false;
TableSizeStats.java
    public boolean tableStats = false;
    public boolean partitionStats = false;
    public Boolean help = false;
HoodieMultiTableServicesMain.java
    public boolean autoDiscovery = false;
    public boolean batch = false;
    public Boolean help = false;
    public Boolean enableCompaction = false;
    public Boolean enableClustering = false;
    public Boolean enableClean = false;
    public Boolean enableArchive = false;
TimelineServerPerf.java
    public Boolean waitForManualQueries = false;
    public Boolean help = false;
HoodieMultiTableStreamer.java
    public Boolean filterDupes = false;
    public Boolean enableHiveSync = false;
    public Boolean enableMetaSync = false;
    public Boolean continuousMode = false;
    public Boolean commitOnErrors = false;
    public Boolean forceDisableCompaction = false;
    public Boolean help = false;
HoodieStreamer.java
    public Boolean enableHiveSync = false;
    public Boolean enableMetaSync = false;
    public Boolean forceEmptyMetaSync = false;
    public Boolean continuousMode = false;
    public Boolean commitOnErrors = false;
    public Boolean forceDisableCompaction = false;
    public Boolean runBootstrap = false;
    public Boolean bootstrapOverwrite = false;
    public Boolean retryOnSourceFailures = false;
    public Boolean allowCommitOnNoCheckpointChange = false;
    public Boolean help = false;
    public Boolean retryLastPendingInlineClusteringJob = false;
    public Boolean retryLastPendingInlineCompactionJob = false;

[vinothchandar]

@wombatu-kun the ones with false make sense , right.. if the user does --continuousMode it becomes true.

issue is the ones that default to true?

[vinothchandar]

@xushiyan can you please actively drive the discussions you re starting. :)

[xushiyan]

Yes @vinothchandar , we only concerned about default true. In addition to @wombatu-kun 's list:

HoodieClusteringJob.java
    public Boolean skipClean = true;
HoodieCompactionAdminTool.java
    public boolean printOutput = true;
HoodieCompactor.java
    public Boolean skipClean = true;
HoodieDropPartitionsTool.java
    public boolean hiveUseJdbc = true;
HoodieMetadataTableValidator.java
   public boolean validateRecordIndexCount = true;
   public boolean validateRecordIndexContent = true;

there are a few sync tool related options that are default true:

--use-file-listing-from-metadata
--sync-incremental
--use-jdbc
--auto-create-database
--spark-datasource

Approach 1: keep behaviors compatible

Keep everything as is, tweak the option parsing to accept --a-boolean-flag true|false as additional format.

Approach 2: breaking changes

This approach is about rename, drop, or change default for applicable options, reducing number of options.

For

HoodieClusteringJob.java
    public Boolean skipClean = true;
HoodieCompactor.java
    public Boolean skipClean = true;

Rename skipClean to doClean and set default false

For

HoodieCompactionAdminTool.java
    public boolean printOutput = true;

Drop the option, always print output.

For

HoodieDropPartitionsTool.java
    public boolean hiveUseJdbc = true;

--use-jdbc (sync tool)

drop the option, using HMS sync mode as default

For

HoodieMetadataTableValidator.java
   public boolean validateRecordIndexCount = true;
   public boolean validateRecordIndexContent = true;

remove the all options like validateXXX, use a new option --validation-list X,Y,Z to pass comma-delimited options to configure what to validate

For

--auto-create-database
--use-file-listing-from-metadata
--sync-incremental
--spark-datasource

drop these; always perform these behaviors:

• sync tool always tries to create database if not exist as per the database name config
• If metadata is not available, then file listing auto fallback to fs listing.
• Add an option to allow full sync --full-sync and default false.
• always sync as spark datasource table to HMS; add an option --sync-as-hive-table and default false

All these are breaking changes that should be flagged in a major release that has these.

If approach 1 is doable, then anyway can go for it. For those changes listed in the approach 2, we can select the changes that make more sense and improve usability, then implement and flag them in the release. Please share your thoughts.