keith-turner commented on code in PR #5340:
URL: https://github.com/apache/accumulo/pull/5340#discussion_r2049317014


##########
core/src/main/java/org/apache/accumulo/core/metadata/schema/TabletMetadata.java:
##########
@@ -558,6 +559,26 @@ private static Optional<TServerInstance> 
checkServer(ClientContext context, Stri
         .map(address -> new TServerInstance(address, 
stat.getEphemeralOwner()));
   }
 
+  public static void validate(TabletMetadata tm) {
+    if (!tm.fetchedCols.contains(ColumnType.FILES) || !tm.sawPrevEndRow) {

Review Comment:
   Maybe part of the problem is that the test are using `new 
TabletMetadata.Builder();` which is a package private class that is only 
visible in the test because its in the same package.  Most test code uses 
`TabletMetadata.build(KeyExtent extent);` which is public and works very 
differently.   When using `new TabletMetadata.Builder();` its internal impl 
code that expected to be used in conjunction with 
`TabletMetadata.convertRow()`.  When using `TabletMetadata.build(KeyExtent 
extent);` it will call `TabletMetadata.convertRow()` which will call `new 
TabletMetadata.Builder();`.
   
   This is very confusing, took a bit to figure out what was going on here.   
Most of the time when writing test `TabletMetadata.Builder()` wil not be 
visible so there are not two confusing choices.  Because of the package this 
test is in, will see two different possible ways to do this.  Not a change for 
this PR, but we should probably rename `TabletMetadata.Builder` to something 
like `TabletMetadata.BuilderImpl`. 
   
   For the new unit test in this PR it would probably be good to use 
`TabletMetadata.build(KeyExtent extent)` instead of ` 
TabletMetadata.Builder()`.  Doing that the code will get more realistic 
TabletMetadata objects that have gone through the same process that would 
happen when building a TabletMetadata object from Keys and Values.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to