[hbase] branch branch-2 updated: HBASE-25238 Upgrading HBase from 2.2.0 to 2.3.x fails because of “Message missing required fields: state” (#2625)

[st...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

stack pushed a commit to branch branch-2
in repository https://gitbox.apache.org/repos/asf/hbase.git


The following commit(s) were added to refs/heads/branch-2 by this push:
     new df0009d  HBASE-25238 Upgrading HBase from 2.2.0 to 2.3.x fails because of “Message missing required fields: state” (#2625)
df0009d is described below

commit df0009d879c6d725911faf2de13726d61dc6bcc3
Author: Michael Stack <sa...@users.noreply.github.com>
AuthorDate: Thu Nov 5 08:36:55 2020 -0800

    HBASE-25238 Upgrading HBase from 2.2.0 to 2.3.x fails because of “Message missing required fields: state” (#2625)
    
    Make protobuf fields add post-2.0.0 release marked 'required' instead
    be 'optional' so migrations from 2.0.x to 2.1+ or 2.2+ succeeds.
---
 .../java/org/apache/hadoop/hbase/shaded/protobuf/ProtobufUtil.java | 4 ++--
 hbase-protocol-shaded/src/main/protobuf/ClusterStatus.proto        | 7 +++++--
 hbase-protocol-shaded/src/main/protobuf/MasterProcedure.proto      | 4 +++-
 .../hadoop/hbase/master/assignment/RegionRemoteProcedureBase.java  | 5 ++++-
 4 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/shaded/protobuf/ProtobufUtil.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/shaded/protobuf/ProtobufUtil.java
index f76b180..a56c90c 100644
--- a/hbase-client/src/main/java/org/apache/hadoop/hbase/shaded/protobuf/ProtobufUtil.java
+++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/shaded/protobuf/ProtobufUtil.java
@@ -2755,8 +2755,8 @@ public final class ProtobufUtil {
       ClusterStatusProtos.ReplicationLoadSink rls) {
     return new ReplicationLoadSink(rls.getAgeOfLastAppliedOp(),
         rls.getTimeStampsOfLastAppliedOp(),
-        rls.getTimestampStarted(),
-        rls.getTotalOpsProcessed());
+        rls.hasTimestampStarted()? rls.getTimestampStarted(): -1L,
+        rls.hasTotalOpsProcessed()? rls.getTotalOpsProcessed(): -1L);
   }
 
   public static ReplicationLoadSource toReplicationLoadSource(
diff --git a/hbase-protocol-shaded/src/main/protobuf/ClusterStatus.proto b/hbase-protocol-shaded/src/main/protobuf/ClusterStatus.proto
index f9af2fc..acff682 100644
--- a/hbase-protocol-shaded/src/main/protobuf/ClusterStatus.proto
+++ b/hbase-protocol-shaded/src/main/protobuf/ClusterStatus.proto
@@ -207,8 +207,11 @@ message ClientMetrics {
 message ReplicationLoadSink {
   required uint64 ageOfLastAppliedOp = 1;
   required uint64 timeStampsOfLastAppliedOp = 2;
-  required uint64 timestampStarted = 3;
-  required uint64 totalOpsProcessed = 4;
+  // The below two were added after hbase-2.0.0 went out. They have to be added as 'optional' else
+  // we break upgrades; old RegionServers reporting in w/ old forms of this message will fail to
+  // deserialize on the new Master. See HBASE-25234
+  optional uint64 timestampStarted = 3;
+  optional uint64 totalOpsProcessed = 4;
 }
 
 message ReplicationLoadSource {
diff --git a/hbase-protocol-shaded/src/main/protobuf/MasterProcedure.proto b/hbase-protocol-shaded/src/main/protobuf/MasterProcedure.proto
index c852bf3..e060077 100644
--- a/hbase-protocol-shaded/src/main/protobuf/MasterProcedure.proto
+++ b/hbase-protocol-shaded/src/main/protobuf/MasterProcedure.proto
@@ -510,7 +510,9 @@ enum RegionRemoteProcedureBaseState {
 message RegionRemoteProcedureBaseStateData {
   required RegionInfo region = 1;
   required ServerName target_server = 2;
-  required RegionRemoteProcedureBaseState state = 3;
+  // state is actually 'required' but we can't set it as 'required' here else it breaks old
+  // Messages; see HBASE-22074.
+  optional RegionRemoteProcedureBaseState state = 3;
   optional RegionStateTransition.TransitionCode transition_code = 4;
   optional int64 seq_id = 5;
 }
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/RegionRemoteProcedureBase.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/RegionRemoteProcedureBase.java
index 1c90d81..805b51c 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/RegionRemoteProcedureBase.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/RegionRemoteProcedureBase.java
@@ -352,7 +352,10 @@ public abstract class RegionRemoteProcedureBase extends Procedure<MasterProcedur
       serializer.deserialize(RegionRemoteProcedureBaseStateData.class);
     region = ProtobufUtil.toRegionInfo(data.getRegion());
     targetServer = ProtobufUtil.toServerName(data.getTargetServer());
-    state = data.getState();
+    // 'state' may not be present if we are reading an 'old' form of this pb Message.
+    if (data.hasState()) {
+      state = data.getState();
+    }
     if (data.hasTransitionCode()) {
       transitionCode = data.getTransitionCode();
       seqId = data.getSeqId();