sharmaar12 commented on code in PR #7149:
URL: https://github.com/apache/hbase/pull/7149#discussion_r2307682598
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java:
##########
@@ -4543,4 +4544,61 @@ protected String getDescription() {
}
});
}
+
+ public Long refreshHfiles(final TableName tableName, final long nonceGroup,
final long nonce)
+ throws IOException {
+ // TODO Check if table exists otherwise send exception.
+ // return 121L;
Review Comment:
Done.
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/RefreshHFilesRegionProcedure.java:
##########
@@ -0,0 +1,132 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.master.procedure;
+
+import java.io.IOException;
+import java.util.Optional;
+import org.apache.hadoop.hbase.ServerName;
+import org.apache.hadoop.hbase.TableName;
+import org.apache.hadoop.hbase.client.RegionInfo;
+import org.apache.hadoop.hbase.master.assignment.RegionStateNode;
+import org.apache.hadoop.hbase.master.assignment.RegionStates;
+import org.apache.hadoop.hbase.procedure2.FailedRemoteDispatchException;
+import org.apache.hadoop.hbase.procedure2.Procedure;
+import org.apache.hadoop.hbase.procedure2.ProcedureStateSerializer;
+import org.apache.hadoop.hbase.procedure2.ProcedureSuspendedException;
+import org.apache.hadoop.hbase.procedure2.ProcedureYieldException;
+import org.apache.hadoop.hbase.procedure2.RemoteProcedureDispatcher;
+import org.apache.hadoop.hbase.procedure2.RemoteProcedureException;
+import org.apache.hadoop.hbase.regionserver.RefreshHFilesCallable;
+import org.apache.yetus.audience.InterfaceAudience;
+
+import org.apache.hadoop.hbase.shaded.protobuf.ProtobufUtil;
+import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProcedureProtos;
+
[email protected]
+public class RefreshHFilesRegionProcedure extends Procedure<MasterProcedureEnv>
+ implements TableProcedureInterface,
+ RemoteProcedureDispatcher.RemoteProcedure<MasterProcedureEnv, ServerName> {
+ private RegionInfo region;
+
+ public RefreshHFilesRegionProcedure() {
+ }
+
+ public RefreshHFilesRegionProcedure(RegionInfo region) {
+ this.region = region;
+ }
+
+ @Override
+ protected void deserializeStateData(ProcedureStateSerializer serializer)
throws IOException {
+ MasterProcedureProtos.RefreshHFilesRegionProcedureStateData data =
+
serializer.deserialize(MasterProcedureProtos.RefreshHFilesRegionProcedureStateData.class);
+ this.region = ProtobufUtil.toRegionInfo(data.getRegion());
+ // TODO Get the Data from region server
+ }
+
+ @Override
+ protected void serializeStateData(ProcedureStateSerializer serializer)
throws IOException {
+ MasterProcedureProtos.RefreshHFilesRegionProcedureStateData.Builder
builder =
+ MasterProcedureProtos.RefreshHFilesRegionProcedureStateData.newBuilder();
+ builder.setRegion(ProtobufUtil.toRegionInfo(region));
+ // TODO add data that you want to pass to region server
Review Comment:
Done.
##########
hbase-client/src/main/java/org/apache/hadoop/hbase/client/RawAsyncHBaseAdmin.java:
##########
@@ -4557,4 +4559,49 @@ List<String>> adminCall(controller, stub,
request.build(),
resp -> resp.getCachedFilesList()))
.serverName(serverName).call();
}
+
+ @Override
+ public CompletableFuture<Long> refreshHFiles(final TableName tableName) {
+ // Request builder
+ RefreshHFilesRequest.Builder request = RefreshHFilesRequest.newBuilder();
+ request.setTableName(ProtobufUtil.toProtoTableName(tableName));
Review Comment:
Yes. What @anmolnar said is correct if user want to refresh hfiles for a
table under specific namespace he will use command like `refresh_hfiles
"TABLE_NAME" => "test_ns:test"`, when no namespace is prefixed then default is
assumed.
##########
hbase-protocol-shaded/src/main/protobuf/server/master/MasterProcedure.proto:
##########
@@ -821,3 +821,20 @@ enum CloseTableRegionsProcedureState {
message CloseTableRegionsProcedureStateData {
required TableName table_name = 1;
}
+
+enum RefreshHFilesTableProcedureState {
+ REFRESH_HFILES_PREPARE = 1;
+ REFRESH_HFILES_REFRESH_REGION = 2;
+ REFRESH_HFILES_FINISH = 3;
+}
+
+message RefreshHFilesTableProcedureStateData {
+}
Review Comment:
Added the code related to this in the latest commits.
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterRpcServices.java:
##########
@@ -3661,4 +3661,28 @@ public FlushTableResponse flushTable(RpcController
controller, FlushTableRequest
throw new ServiceException(ioe);
}
}
+
+ @Override
+ public MasterProtos.RefreshHFilesResponse refreshHFiles(RpcController
controller,
+ MasterProtos.RefreshHFilesRequest request) throws ServiceException {
+ // TODO Check if table exists otherwise send exception.
Review Comment:
Done. Added code to handle the exception cases in HMaster instead of
MasterRpcServices. Removed redundant TODOs.
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/RefreshHFilesRegionProcedure.java:
##########
@@ -0,0 +1,132 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.master.procedure;
+
+import java.io.IOException;
+import java.util.Optional;
+import org.apache.hadoop.hbase.ServerName;
+import org.apache.hadoop.hbase.TableName;
+import org.apache.hadoop.hbase.client.RegionInfo;
+import org.apache.hadoop.hbase.master.assignment.RegionStateNode;
+import org.apache.hadoop.hbase.master.assignment.RegionStates;
+import org.apache.hadoop.hbase.procedure2.FailedRemoteDispatchException;
+import org.apache.hadoop.hbase.procedure2.Procedure;
+import org.apache.hadoop.hbase.procedure2.ProcedureStateSerializer;
+import org.apache.hadoop.hbase.procedure2.ProcedureSuspendedException;
+import org.apache.hadoop.hbase.procedure2.ProcedureYieldException;
+import org.apache.hadoop.hbase.procedure2.RemoteProcedureDispatcher;
+import org.apache.hadoop.hbase.procedure2.RemoteProcedureException;
+import org.apache.hadoop.hbase.regionserver.RefreshHFilesCallable;
+import org.apache.yetus.audience.InterfaceAudience;
+
+import org.apache.hadoop.hbase.shaded.protobuf.ProtobufUtil;
+import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProcedureProtos;
+
[email protected]
+public class RefreshHFilesRegionProcedure extends Procedure<MasterProcedureEnv>
+ implements TableProcedureInterface,
+ RemoteProcedureDispatcher.RemoteProcedure<MasterProcedureEnv, ServerName> {
+ private RegionInfo region;
+
+ public RefreshHFilesRegionProcedure() {
+ }
+
+ public RefreshHFilesRegionProcedure(RegionInfo region) {
+ this.region = region;
+ }
+
+ @Override
+ protected void deserializeStateData(ProcedureStateSerializer serializer)
throws IOException {
+ MasterProcedureProtos.RefreshHFilesRegionProcedureStateData data =
+
serializer.deserialize(MasterProcedureProtos.RefreshHFilesRegionProcedureStateData.class);
+ this.region = ProtobufUtil.toRegionInfo(data.getRegion());
+ // TODO Get the Data from region server
Review Comment:
Done.
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterRpcServices.java:
##########
@@ -3661,4 +3661,28 @@ public FlushTableResponse flushTable(RpcController
controller, FlushTableRequest
throw new ServiceException(ioe);
}
}
+
+ @Override
+ public MasterProtos.RefreshHFilesResponse refreshHFiles(RpcController
controller,
+ MasterProtos.RefreshHFilesRequest request) throws ServiceException {
+ // TODO Check if table exists otherwise send exception.
+ try {
+ Long procId;
+ if (request.hasTableName()) { // if we have provided table name as
parameter
Review Comment:
When both tablename and namespace is specified, for example, `refresh_hfiles
"NAMESPACE" => "test_ns", "TABLE_NAME" => "test"`, then we are raising
ArgumentError in ruby script to avoid confusion. As there are two ways to
interpret this. First is refresh hfiles for all tables under namespace
`test_ns` and refresh hfiles for table `test`. Second is refresh hfiles for
table test under namespace `test_ns`.
If user want to refresh hfiles for table test under namespace test_ns he has
to use command as `refresh_hfiles "TABLE_NAME" => "test_ns:test"`, which is
mostly followed in other hbase commands. If no namespace is provided then
default is assumed.
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/RefreshHFilesRegionProcedure.java:
##########
@@ -0,0 +1,132 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.master.procedure;
+
+import java.io.IOException;
+import java.util.Optional;
+import org.apache.hadoop.hbase.ServerName;
+import org.apache.hadoop.hbase.TableName;
+import org.apache.hadoop.hbase.client.RegionInfo;
+import org.apache.hadoop.hbase.master.assignment.RegionStateNode;
+import org.apache.hadoop.hbase.master.assignment.RegionStates;
+import org.apache.hadoop.hbase.procedure2.FailedRemoteDispatchException;
+import org.apache.hadoop.hbase.procedure2.Procedure;
+import org.apache.hadoop.hbase.procedure2.ProcedureStateSerializer;
+import org.apache.hadoop.hbase.procedure2.ProcedureSuspendedException;
+import org.apache.hadoop.hbase.procedure2.ProcedureYieldException;
+import org.apache.hadoop.hbase.procedure2.RemoteProcedureDispatcher;
+import org.apache.hadoop.hbase.procedure2.RemoteProcedureException;
+import org.apache.hadoop.hbase.regionserver.RefreshHFilesCallable;
+import org.apache.yetus.audience.InterfaceAudience;
+
+import org.apache.hadoop.hbase.shaded.protobuf.ProtobufUtil;
+import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProcedureProtos;
+
[email protected]
+public class RefreshHFilesRegionProcedure extends Procedure<MasterProcedureEnv>
+ implements TableProcedureInterface,
+ RemoteProcedureDispatcher.RemoteProcedure<MasterProcedureEnv, ServerName> {
+ private RegionInfo region;
+
+ public RefreshHFilesRegionProcedure() {
+ }
+
+ public RefreshHFilesRegionProcedure(RegionInfo region) {
+ this.region = region;
+ }
+
+ @Override
+ protected void deserializeStateData(ProcedureStateSerializer serializer)
throws IOException {
+ MasterProcedureProtos.RefreshHFilesRegionProcedureStateData data =
+
serializer.deserialize(MasterProcedureProtos.RefreshHFilesRegionProcedureStateData.class);
+ this.region = ProtobufUtil.toRegionInfo(data.getRegion());
+ // TODO Get the Data from region server
+ }
+
+ @Override
+ protected void serializeStateData(ProcedureStateSerializer serializer)
throws IOException {
+ MasterProcedureProtos.RefreshHFilesRegionProcedureStateData.Builder
builder =
+ MasterProcedureProtos.RefreshHFilesRegionProcedureStateData.newBuilder();
+ builder.setRegion(ProtobufUtil.toRegionInfo(region));
+ // TODO add data that you want to pass to region server
+ serializer.serialize(builder.build());
+ }
+
+ @Override
+ protected boolean abort(MasterProcedureEnv env) {
+ return false;
+ }
+
+ @Override
+ protected void rollback(MasterProcedureEnv env) throws IOException,
InterruptedException {
+ throw new UnsupportedOperationException();
+ }
+
+ @Override
+ protected Procedure<MasterProcedureEnv>[] execute(MasterProcedureEnv env)
+ throws ProcedureYieldException, ProcedureSuspendedException,
InterruptedException {
+ RegionStates regionStates = env.getAssignmentManager().getRegionStates();
+ RegionStateNode regionNode = regionStates.getRegionStateNode(region);
Review Comment:
Added the code related to this in latest commit.
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/RefreshHFilesRegionProcedure.java:
##########
@@ -0,0 +1,132 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.master.procedure;
+
+import java.io.IOException;
+import java.util.Optional;
+import org.apache.hadoop.hbase.ServerName;
+import org.apache.hadoop.hbase.TableName;
+import org.apache.hadoop.hbase.client.RegionInfo;
+import org.apache.hadoop.hbase.master.assignment.RegionStateNode;
+import org.apache.hadoop.hbase.master.assignment.RegionStates;
+import org.apache.hadoop.hbase.procedure2.FailedRemoteDispatchException;
+import org.apache.hadoop.hbase.procedure2.Procedure;
+import org.apache.hadoop.hbase.procedure2.ProcedureStateSerializer;
+import org.apache.hadoop.hbase.procedure2.ProcedureSuspendedException;
+import org.apache.hadoop.hbase.procedure2.ProcedureYieldException;
+import org.apache.hadoop.hbase.procedure2.RemoteProcedureDispatcher;
+import org.apache.hadoop.hbase.procedure2.RemoteProcedureException;
+import org.apache.hadoop.hbase.regionserver.RefreshHFilesCallable;
+import org.apache.yetus.audience.InterfaceAudience;
+
+import org.apache.hadoop.hbase.shaded.protobuf.ProtobufUtil;
+import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProcedureProtos;
+
[email protected]
+public class RefreshHFilesRegionProcedure extends Procedure<MasterProcedureEnv>
+ implements TableProcedureInterface,
+ RemoteProcedureDispatcher.RemoteProcedure<MasterProcedureEnv, ServerName> {
+ private RegionInfo region;
+
+ public RefreshHFilesRegionProcedure() {
+ }
+
+ public RefreshHFilesRegionProcedure(RegionInfo region) {
+ this.region = region;
+ }
+
+ @Override
+ protected void deserializeStateData(ProcedureStateSerializer serializer)
throws IOException {
+ MasterProcedureProtos.RefreshHFilesRegionProcedureStateData data =
+
serializer.deserialize(MasterProcedureProtos.RefreshHFilesRegionProcedureStateData.class);
+ this.region = ProtobufUtil.toRegionInfo(data.getRegion());
+ // TODO Get the Data from region server
+ }
+
+ @Override
+ protected void serializeStateData(ProcedureStateSerializer serializer)
throws IOException {
+ MasterProcedureProtos.RefreshHFilesRegionProcedureStateData.Builder
builder =
+ MasterProcedureProtos.RefreshHFilesRegionProcedureStateData.newBuilder();
+ builder.setRegion(ProtobufUtil.toRegionInfo(region));
+ // TODO add data that you want to pass to region server
+ serializer.serialize(builder.build());
+ }
+
+ @Override
+ protected boolean abort(MasterProcedureEnv env) {
+ return false;
+ }
+
+ @Override
+ protected void rollback(MasterProcedureEnv env) throws IOException,
InterruptedException {
+ throw new UnsupportedOperationException();
+ }
+
+ @Override
+ protected Procedure<MasterProcedureEnv>[] execute(MasterProcedureEnv env)
+ throws ProcedureYieldException, ProcedureSuspendedException,
InterruptedException {
+ RegionStates regionStates = env.getAssignmentManager().getRegionStates();
+ RegionStateNode regionNode = regionStates.getRegionStateNode(region);
+
+ ServerName targetServer = regionNode.getRegionLocation();
+
+ try {
+ env.getRemoteDispatcher().addOperationToNode(targetServer, this);
+ } catch (FailedRemoteDispatchException e) {
+ throw new ProcedureSuspendedException();
+ }
+
+ return null;
+ }
+
+ @Override
+ public TableOperationType getTableOperationType() {
+ return TableOperationType.REFRESH_HFILES;
+ }
+
+ @Override
+ public TableName getTableName() {
+ return region.getTable();
+ }
+
+ @Override
+ public void remoteOperationFailed(MasterProcedureEnv env,
RemoteProcedureException error) {
+ // TODO redo the same thing again till retry count else send the error to
client.
Review Comment:
Added the code to address these TODOs in latest commits.
--
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]
