-
Notifications
You must be signed in to change notification settings - Fork 50
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
(TEPHRA-214) Tool to debug the state and progress of Invalid List Pruning #31
(TEPHRA-214) Tool to debug the state and progress of Invalid List Pruning #31
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@gokulavasan I took a pass over this, I have a few comments.
} | ||
|
||
public static void main(String[] args) { | ||
Configuration hConf = new Configuration(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be HBaseConfiguration.create()
, right?
@@ -147,6 +147,7 @@ public long fetchPruneUpperBound(long time, long inactiveTransactionBound) throw | |||
|
|||
// Get all the current transactional regions | |||
SortedSet<byte[]> transactionalRegions = getTransactionalRegions(); | |||
// TODO: Ignore empty |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This TODO needs to be handled
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That TODO was meant for ignoring empty regions from the current transactional regions. I have removed that misleading TODO as part of the last commit.
* @throws IOException | ||
*/ | ||
public void initialize(final Configuration conf) throws IOException { | ||
LOG.info("InvalidListPruningDebugMain : initialize method called"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can be changed to debug log
pw.println("Usage : org.apache.tephra.hbase.txprune.InvalidListPruning <command> <parameter>"); | ||
pw.println("Available commands, corresponding parameters are:"); | ||
pw.println("****************************************************"); | ||
pw.println("timeregion ts"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we make it time-region
? It is more readable
pw.println("****************************************************"); | ||
pw.println("timeregion ts"); | ||
pw.println("Desc: Prints out a time region at time 'ts' (in milliseconds) or the latest before time 'ts'."); | ||
pw.println("idleregions numOfRegions"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here idle-regions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also numOfRegions
is a maximum limit, right? Maybe call it limit
?
@@ -102,11 +107,29 @@ public void savePruneUpperBoundForRegion(byte[] regionId, long pruneUpperBound) | |||
* @throws IOException when not able to read the data from HBase | |||
*/ | |||
public long getPruneUpperBoundForRegion(byte[] regionId) throws IOException { | |||
RegionPruneInfo regionPruneInfo = getPruneInfoForRegion(regionId); | |||
return (regionPruneInfo == null) ? -1 : regionPruneInfo.getCompactionTimestamp(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This has to return the regionPruneInfo.getPruneUpperBound()
, right?
* @return {@link RegionPruneInfo} for the region | ||
* @throws IOException when not able to read the data from HBase | ||
*/ | ||
public RegionPruneInfo getPruneInfoForRegion(byte[] regionId) throws IOException { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This method needs to be annotated as @Nullable
@@ -120,6 +143,22 @@ public long getPruneUpperBoundForRegion(byte[] regionId) throws IOException { | |||
*/ | |||
public Map<byte[], Long> getPruneUpperBoundForRegions(SortedSet<byte[]> regions) throws IOException { | |||
Map<byte[], Long> resultMap = new TreeMap<>(Bytes.BYTES_COMPARATOR); | |||
List<RegionPruneInfo> regionPruneInfos = getPruneInfoForRegions(regions); | |||
for (RegionPruneInfo regionPruneInfo : regionPruneInfos) { | |||
resultMap.put(regionPruneInfo.getRegionName(), regionPruneInfo.getCompactionTimestamp()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here, should be regionPruneInfo.getPruneUpperBound()
instead of regionPruneInfo.getCompactionTimestamp()
Cell cell = next.getColumnLatestCell(FAMILY, PRUNE_UPPER_BOUND_COL); | ||
if (cell != null) { | ||
byte[] pruneUpperBoundBytes = CellUtil.cloneValue(cell); | ||
long timestamp = cell.getTimestamp(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not compaction timestamp, this is when the prune upper bound was written to HBase. Although this can be an approximate value when the state table is available. Is the compaction timestamp really necessary for debugging?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also it may be okay to treat this as the compaction timestamp for now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@poornachandra It might be useful information to have right? I can rename it to something else if you think compaction timestamp is misleading.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What would be a good name for this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PruneRecordTime ?
pw.println("Available commands, corresponding parameters are:"); | ||
pw.println("****************************************************"); | ||
pw.println("timeregion ts"); | ||
pw.println("Desc: Prints out a time region at time 'ts' (in milliseconds) or the latest before time 'ts'."); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Prints out the transactional regions present in HBase at time...
LGTM |
e7d7db4
to
acd3395
Compare
@poornachandra Ported changes to other modules. Please take a look. Thanks! |
LGTM |
5e2b3c5
to
34f58ee
Compare
JIRA : https://issues.apache.org/jira/browse/TEPHRA-214
Summary: Introduce a debug tool to fetch data that might help with debugging the status and progress of invalid list transaction list pruning.
Three commands are supported:
timeregion ==> Will return TimeRegions at or before timestamp in millis
idleregions ==> Will return numOfRegions regions with lowest pruneUpperBounds. If -1 is provided, all pruneUpperBounds are returned.
pruneinfo => Will return pruneUpperBound, compactionTimeStamp of the 'regionNameAsString' region.
This tool can be invoked with hbase classpath and tephra jars in the classpath.