taklwu commented on code in PR #7037:
URL: https://github.com/apache/hbase/pull/7037#discussion_r2112573576
##########
hbase-diagnostics/src/main/java/org/apache/hadoop/hbase/PerformanceEvaluation.java:
##########
@@ -367,36 +367,67 @@ public void setStatus(String msg) {
* opts.replicas}.
*/
static boolean checkTable(Admin admin, TestOptions opts) throws IOException {
- TableName tableName = TableName.valueOf(opts.tableName);
- boolean needsDelete = false, exists = admin.tableExists(tableName);
- boolean isReadCmd = opts.cmdName.toLowerCase(Locale.ROOT).contains("read")
+ final TableName tableName = TableName.valueOf(opts.tableName);
+ final boolean exists = admin.tableExists(tableName);
+ final boolean isReadCmd =
opts.cmdName.toLowerCase(Locale.ROOT).contains("read")
|| opts.cmdName.toLowerCase(Locale.ROOT).contains("scan");
- boolean isDeleteCmd =
opts.cmdName.toLowerCase(Locale.ROOT).contains("delete");
- if (!exists && (isReadCmd || isDeleteCmd)) {
+ final boolean isDeleteCmd =
opts.cmdName.toLowerCase(Locale.ROOT).contains("delete");
+ final boolean needsData = isReadCmd || isDeleteCmd;
+ if (!exists && needsData) {
throw new IllegalStateException(
- "Must specify an existing table for read commands. Run a write command
first.");
+ "Must specify an existing table for read/delete commands. Run a write
command first.");
}
TableDescriptor desc = exists ?
admin.getDescriptor(TableName.valueOf(opts.tableName)) : null;
- byte[][] splits = getSplits(opts);
+ final byte[][] splits = getSplits(opts);
// recreate the table when user has requested presplit or when existing
// {RegionSplitPolicy,replica count} does not match requested, or when the
// number of column families does not match requested.
- if (
- (exists && opts.presplitRegions != DEFAULT_OPTS.presplitRegions
- && opts.presplitRegions != admin.getRegions(tableName).size())
- || (!isReadCmd && desc != null
- && !StringUtils.equals(desc.getRegionSplitPolicyClassName(),
opts.splitPolicy))
- || (!(isReadCmd || isDeleteCmd) && desc != null
- && desc.getRegionReplication() != opts.replicas)
- || (desc != null && desc.getColumnFamilyCount() != opts.families)
- ) {
- needsDelete = true;
- // wait, why did it delete my table?!?
- LOG.debug(MoreObjects.toStringHelper("needsDelete").add("needsDelete",
needsDelete)
- .add("isReadCmd", isReadCmd).add("exists", exists).add("desc", desc)
- .add("presplit", opts.presplitRegions).add("splitPolicy",
opts.splitPolicy)
- .add("replicas", opts.replicas).add("families",
opts.families).toString());
+ final boolean regionCountChanged =
+ exists && opts.presplitRegions != DEFAULT_OPTS.presplitRegions
+ && opts.presplitRegions != admin.getRegions(tableName).size();
+ final boolean splitPolicyChanged =
+ exists && !StringUtils.equals(desc.getRegionSplitPolicyClassName(),
opts.splitPolicy);
+ final boolean regionReplicationChanged = exists &&
desc.getRegionReplication() != opts.replicas;
+ final boolean columnFamilyCountChanged = exists &&
desc.getColumnFamilyCount() != opts.families;
+
+ boolean needsDelete = regionCountChanged || splitPolicyChanged ||
regionReplicationChanged
+ || columnFamilyCountChanged;
+
+ if (needsDelete) {
+ final List<String> errors = new ArrayList<>();
+ if (columnFamilyCountChanged) {
+ final String error = String.format("--families=%d, but found %d column
families",
+ opts.families, desc.getColumnFamilyCount());
+ if (needsData) {
+ // We can't proceed the test in this case
+ throw new IllegalStateException(
+ "Cannot proceed the test. Run a write command first: " + error);
+ }
+ errors.add(error);
+ }
+ if (regionCountChanged) {
+ errors.add(String.format("--presplit=%d, but found %d regions",
opts.presplitRegions,
+ admin.getRegions(tableName).size()));
+ }
+ if (splitPolicyChanged) {
+ errors.add(String.format("--splitPolicy=%s, but current policy is %s",
opts.splitPolicy,
+ desc.getRegionSplitPolicyClassName()));
+ }
+ if (regionReplicationChanged) {
+ errors.add(String.format("--replicas=%d, but found %d replicas",
opts.replicas,
+ desc.getRegionReplication()));
+ }
+ final String reason =
+ errors.stream().map(s -> '[' + s + ']').collect(Collectors.joining(",
"));
+
+ if (needsData) {
+ LOG.warn(
+ "Inconsistent table state detected. Consider running a write command
first: " + reason);
+ needsDelete = false;
Review Comment:
correct me if I'm wrong, it sounds like some wrapper script accidentally
added `--presplit=30` to the read command? still, I like how the new warning
message idea and code refactoring definitely help make the read logic more
robust.
--
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: issues-unsubscr...@hbase.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
