You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@accumulo.apache.org by "ArbaazKhan1 (via GitHub)" <gi...@apache.org> on 2026/08/27 23:25:19 UTC
[PR] Human-readable metadata scan [accumulo]
ArbaazKhan1 opened a new pull request, #6519:
URL: https://github.com/apache/accumulo/pull/6519
Closes [issue #3772 ](https://github.com/apache/accumulo/issues/3772)
- Updated `StoredTabletFile.toString` to use `toMinimalString()` making tablet file ranges readable in logs and other outputs.
- Updated `ScanCommand` to decode file and scan column qualifiers when scanning metadata tables
- cleaned up `FileMetadataUtil` loggin to use SLF4J placeholders
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Human-readable metadata scan [accumulo]
Posted by "dlmarion (via GitHub)" <gi...@apache.org>.
dlmarion commented on code in PR #6519:
URL: https://github.com/apache/accumulo/pull/6519#discussion_r3894600830
##########
test/src/main/java/org/apache/accumulo/test/util/FileMetadataUtil.java:
##########
@@ -68,9 +68,8 @@ public class FileMetadataUtil {
StoredTabletFile file = fileEntry.getKey();
DataFileValue dfv = fileEntry.getValue();
files.put(file, dfv);
- 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,
Review 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()`?
##########
shell/src/main/java/org/apache/accumulo/shell/commands/ScanCommand.java:
##########
@@ -352,6 +400,8 @@ public Options getOptions() {
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,
Review 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.
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Human-readable metadata scan [accumulo]
Posted by "phrocker (via GitHub)" <gi...@apache.org>.
phrocker commented on code in PR #6519:
URL: https://github.com/apache/accumulo/pull/6519#discussion_r3894748436
##########
core/src/main/java/org/apache/accumulo/core/metadata/StoredTabletFile.java:
##########
@@ -150,7 +150,7 @@ public int hashCode() {
@Override
public String toString() {
- return metadataEntry;
+ return toMinimalString();
Review 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?
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Human-readable metadata scan [accumulo]
Posted by "dlmarion (via GitHub)" <gi...@apache.org>.
dlmarion commented on code in PR #6519:
URL: https://github.com/apache/accumulo/pull/6519#discussion_r3894994991
##########
shell/src/main/java/org/apache/accumulo/shell/commands/ScanCommand.java:
##########
@@ -352,6 +400,8 @@ public Options getOptions() {
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,
Review Comment:
It's a good idea, but I think the intention is to remove formatters. See https://github.com/apache/accumulo/pull/3265.
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Human-readable metadata scan [accumulo]
Posted by "phrocker (via GitHub)" <gi...@apache.org>.
phrocker commented on code in PR #6519:
URL: https://github.com/apache/accumulo/pull/6519#discussion_r3894737368
##########
shell/src/main/java/org/apache/accumulo/shell/commands/ScanCommand.java:
##########
@@ -352,6 +400,8 @@ public Options getOptions() {
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,
Review Comment:
Doesn't accumulo already ahve formatters? metadataformatter that extends default formatter could be a better option and obviate the need for an option. also could be default?
##########
shell/src/main/java/org/apache/accumulo/shell/commands/ScanCommand.java:
##########
@@ -352,6 +400,8 @@ public Options getOptions() {
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,
Review 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?
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Human-readable metadata scan [accumulo]
Posted by "phrocker (via GitHub)" <gi...@apache.org>.
phrocker commented on code in PR #6519:
URL: https://github.com/apache/accumulo/pull/6519#discussion_r3896969293
##########
shell/src/main/java/org/apache/accumulo/shell/commands/ScanCommand.java:
##########
@@ -352,6 +400,8 @@ public Options getOptions() {
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,
Review 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.
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Human-readable metadata scan [accumulo]
Posted by "phrocker (via GitHub)" <gi...@apache.org>.
phrocker commented on code in PR #6519:
URL: https://github.com/apache/accumulo/pull/6519#discussion_r3896969293
##########
shell/src/main/java/org/apache/accumulo/shell/commands/ScanCommand.java:
##########
@@ -352,6 +400,8 @@ public Options getOptions() {
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,
Review Comment:
Yes, thank you. I encountered this last year sometime when testing 4.0....
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.
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Human-readable metadata scan [accumulo]
Posted by "phrocker (via GitHub)" <gi...@apache.org>.
phrocker commented on code in PR #6519:
URL: https://github.com/apache/accumulo/pull/6519#discussion_r3896957516
##########
core/src/main/java/org/apache/accumulo/core/metadata/StoredTabletFile.java:
##########
@@ -150,7 +150,7 @@ public int hashCode() {
@Override
public String toString() {
- return metadataEntry;
+ return toMinimalString();
Review 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 ?
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Human-readable metadata scan [accumulo]
Posted by "ArbaazKhan1 (via GitHub)" <gi...@apache.org>.
ArbaazKhan1 commented on code in PR #6519:
URL: https://github.com/apache/accumulo/pull/6519#discussion_r3981957192
##########
shell/src/main/java/org/apache/accumulo/shell/commands/ScanCommand.java:
##########
@@ -352,6 +400,8 @@ public Options getOptions() {
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,
Review Comment:
My goal was to avoid changing the normal metadata scan output in a way that might make users think that the decoded form is the actual stored metadata representation. I figured adding a new option, allows for the default behavior to remain unchanged, and users only get the decoded/human-readable form when they explicitly ask for it
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Human-readable metadata scan [accumulo]
Posted by "phrocker (via GitHub)" <gi...@apache.org>.
phrocker commented on code in PR #6519:
URL: https://github.com/apache/accumulo/pull/6519#discussion_r3896957516
##########
core/src/main/java/org/apache/accumulo/core/metadata/StoredTabletFile.java:
##########
@@ -150,7 +150,7 @@ public int hashCode() {
@Override
public String toString() {
- return metadataEntry;
+ return toMinimalString();
Review 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 fidel+ity 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 ?
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Human-readable metadata scan [accumulo]
Posted by "phrocker (via GitHub)" <gi...@apache.org>.
phrocker commented on code in PR #6519:
URL: https://github.com/apache/accumulo/pull/6519#discussion_r3896976856
##########
core/src/main/java/org/apache/accumulo/core/metadata/StoredTabletFile.java:
##########
@@ -150,7 +150,7 @@ public int hashCode() {
@Override
public String toString() {
- return metadataEntry;
+ return toMinimalString();
Review Comment:
I should use AI to comment so I don't have so many typos...
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Human-readable metadata scan [accumulo]
Posted by "dlmarion (via GitHub)" <gi...@apache.org>.
dlmarion commented on code in PR #6519:
URL: https://github.com/apache/accumulo/pull/6519#discussion_r3895019082
##########
core/src/main/java/org/apache/accumulo/core/metadata/StoredTabletFile.java:
##########
@@ -150,7 +150,7 @@ public int hashCode() {
@Override
public String toString() {
- return metadataEntry;
+ return toMinimalString();
Review 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.
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org