Human-readable metadata scan - #6519
Conversation
| log.debug("Extent: " + tabletMetadata.getExtent() + "; File Name: " + file.getFileName() | ||
| + "; Range: " + file.getRange() + "; Entries: " + dfv.getNumEntries() + ", Size: " | ||
| + dfv.getSize()); | ||
| log.debug("Extent: {}; File: {}; Entries: {}; Size: {}", tabletMetadata.getExtent(), file, |
There was a problem hiding this comment.
Was the change above in StoredTabletFile.toString() made to support just using file here in the log output in place of file.getFileName() and file.getRange()?
| executionHintsOpt = new Option(null, "execution-hints", true, "Execution hints map"); | ||
| scanServerOpt = | ||
| new Option("cl", "consistency-level", true, "set consistency level (experimental)"); | ||
| decodeMetadataOpt = new Option("dm", "decode-metadata", false, |
There was a problem hiding this comment.
I'm not sure that we need an option. If the user has the ability to scan the metadata, then we should just show them the un-obfuscated metadata entry.
There was a problem hiding this comment.
Doesn't accumulo already have formatters? metadataformatter that extends default formatter could be a better option and obviate the need for an option. also could be default?
There was a problem hiding this comment.
It's a good idea, but I think the intention is to remove formatters. See #3265.
There was a problem hiding this comment.
Yes, thank you. I encountered this last year sometime when testing 4.0, but I have a very poor memory....
Should this option be flipped? On by default, old method an option just in case someone relies on the encoded form?
I think it's pretty fair to say 4.0 changes the output, though.
| @Override | ||
| public String toString() { | ||
| return metadataEntry; | ||
| return toMinimalString(); |
There was a problem hiding this comment.
Wouldn't this affect every log line and exception message that stringifies . Would this create some kind of diagnostics regression if anyone is consuming logs for alterting? Should this be moved to the call sites that need it?
There was a problem hiding this comment.
That's where I was going with my other comment. If this change was only made to support the logging change, then I think it should be reverted. I don't think logging changes are an issue for the 4.0 release, but I'm more concerned about some code using the output of toString by mistake and the unintended consequences of changing it.
There was a problem hiding this comment.
I was less concerned about programmatic usage and more concerned about logs given how they could be used in cloud deployments -- but I think the end result is the same: if the fidelity is changing as a result of this change would any downstream consumer have to change processes?
If the answer is yes, then is the ROI of this change in its favor ?
There was a problem hiding this comment.
I should use AI to comment so I don't have so many typos...
3 participants
Closes issue #3772
StoredTabletFile.toStringto usetoMinimalString()making tablet file ranges readable in logs and other outputs.ScanCommandto decode file and scan column qualifiers when scanning metadata tablesFileMetadataUtilloggin to use SLF4J placeholders