[GitHub] [accumulo] ctubbsii commented on a diff in pull request #2897: Make Text field transient in BadLocationStateException

[GitBox <gi...@apache.org>]

ctubbsii commented on code in PR #2897:
URL: https://github.com/apache/accumulo/pull/2897#discussion_r958813179


##########
core/src/main/java/org/apache/accumulo/core/metadata/TabletLocationState.java:
##########
@@ -36,7 +36,7 @@ public class TabletLocationState {
 
   public static class BadLocationStateException extends Exception {
     private static final long serialVersionUID = 1L;
-    private Text metadataTableEntry;
+    private transient final Text metadataTableEntry;

Review Comment:
   It feels incorrect to make this transient. The entire point of storing this value is so that it can be retrieved by the `getEncodedEndRow` method. If it is transient and does lose its value upon serializing, then that method will always return null. Nothing in this class protects against that risk by detecting it. The method would happily return null.
   
   At the very least, we could make sure we check that it is non-null upon construction, and then detect that it has been lost in and throw an IllegalStateException in getEncodedEndRow() if the value is null.



-- 
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