Move checks to the MergeCommand class#6431
Conversation
Move the metadata table name check into the shell's MergeCommand and allow it to be ran with a confirmation prompt. Merging the metadata table is a legitimate operation for cleaning up empty split points from delete markers (~del), scan server entries (~sserv), deleted tables or error messages (~err).
| " Warning!!! Merging the " + MetadataTable.NAME + " table incorrectly can result in " | ||
| + "system instability. Are you REALLY sure you want to merge?!?!?!") |
There was a problem hiding this comment.
Merge will lock the metadata table for write and then unhost the tablets in the range for some period of time while the merge occurs. The metadata tablets could be unhosted under normal balance conditions, but the system will try to host them immediately. I certainly think we should add a test case for this in MergeIT or some other merge related IT. And I'm wondering if we should add a merge of the metadata table during some Fate operations in some Fate IT.
There was a problem hiding this comment.
Added test cases for merge operations in MergeIT.
For the FateITs it sounds like you want to see what system interruptions might occur if merging the metadata table happened while other fate operation are ongoing.
There was a problem hiding this comment.
For the FateITs it sounds like you want to see what system interruptions might occur if merging the metadata table happened while other fate operation are ongoing.
Exactly. It should be fine as metadata tablets can become unhosted during normal balancing, but it would be good to test this case since we are changing the behavior of merge for the metadata table late in the development cycle in a patch release. In fact, we may want to add that something to the message to say that it's experimental if only to get users to test it first.
Adds three test cases for merge operations. Metadata table merge Metadata table merge with 1 byte size option. Unsupported root table merge.
ddanielr
force-pushed
the
feature/move-metadata-table-check
branch
from
61219a1 to
6e3a5fa
Compare
| AccumuloConfiguration tableConfig = | ||
| new ConfigurationCopy(client.tableOperations().getConfiguration(opts.tableName)); | ||
| opts.goalSize = tableConfig.getAsBytes(Property.TABLE_SPLIT_THRESHOLD); | ||
| long newGoalSize = tableConfig.getAsBytes(Property.TABLE_SPLIT_THRESHOLD); |
There was a problem hiding this comment.
This code would previously just output a Merging tablets in table <table> to <size> bytes message but would not explicitly state that the size has changed from what was instructed.
The new code change provides a message describing the change in the size selector.
| tablets = TabletsMetadata.builder(context).scanMetadataTable() | ||
| tableId = context.getTableId(tableName); | ||
| DataLevel dataLevel = DataLevel.of(tableId); | ||
| tablets = TabletsMetadata.builder(context).scanTable(dataLevel.metaTable()) |
There was a problem hiding this comment.
Using the dataLevel of the current table instead of .scanMetadataTable allows the Merge Utility to perform merges against the metadata table.
2 participants
Move the metadata table name check into the shell's MergeCommand and allow it to be ran with a confirmation prompt.
Merging the metadata table is a legitimate operation for cleaning up empty split points from delete markers (~del), scan server entries (~sserv), deleted tables or error messages (~err).
Relates to #6414 as the current merge code should be refactored into two distinct utilities.