joshelser commented on a change in pull request #3536: URL: https://github.com/apache/hbase/pull/3536#discussion_r696061323
########## File path: hbase-client/src/main/java/org/apache/hadoop/hbase/client/BalanceRequest.java ########## @@ -0,0 +1,96 @@ +/* + * + * 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.client; + +import org.apache.yetus.audience.InterfaceAudience; +import org.apache.yetus.audience.InterfaceStability; + +/** + * Encapsulates options for executing a run of the Balancer. + */ [email protected] [email protected] +public final class BalanceRequest { + private static final BalanceRequest DEFAULT = BalanceRequest.newBuilder().build(); + + @InterfaceAudience.Public + @InterfaceStability.Evolving + public final static class Builder { Review comment: Should have some javadoc on the Builder too ########## File path: hbase-client/src/main/java/org/apache/hadoop/hbase/client/BalanceRequest.java ########## @@ -0,0 +1,96 @@ +/* + * + * 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.client; + +import org.apache.yetus.audience.InterfaceAudience; +import org.apache.yetus.audience.InterfaceStability; + +/** + * Encapsulates options for executing a run of the Balancer. + */ [email protected] [email protected] +public final class BalanceRequest { Review comment: We should have javadoc on public methods in `IA.Public` classes ########## File path: hbase-client/src/main/java/org/apache/hadoop/hbase/client/BalanceResponse.java ########## @@ -0,0 +1,100 @@ +/* + * + * 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.client; + +import org.apache.yetus.audience.InterfaceAudience; +import org.apache.yetus.audience.InterfaceStability; + +/** + * Response returned from a balancer invocation + */ [email protected] [email protected] +public final class BalanceResponse { + + @InterfaceAudience.Public + @InterfaceStability.Evolving + public final static class Builder { Review comment: Javadoc on builder too, please. ########## File path: hbase-client/src/main/java/org/apache/hadoop/hbase/client/BalanceResponse.java ########## @@ -0,0 +1,100 @@ +/* + * + * 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.client; + +import org.apache.yetus.audience.InterfaceAudience; +import org.apache.yetus.audience.InterfaceStability; + +/** + * Response returned from a balancer invocation + */ [email protected] [email protected] +public final class BalanceResponse { + + @InterfaceAudience.Public + @InterfaceStability.Evolving + public final static class Builder { + private boolean balancerRan; + private int movesCalculated; + private int movesExecuted; + + private Builder() {} + + public Builder setBalancerRan(boolean balancerRan) { + this.balancerRan = balancerRan; + return this; + } + + public Builder setMovesCalculated(int movesCalculated) { + this.movesCalculated = movesCalculated; + return this; + } + + public Builder setMovesExecuted(int movesExecuted) { + this.movesExecuted = movesExecuted; + return this; + } + + public BalanceResponse build() { + return new BalanceResponse(balancerRan, movesCalculated, movesExecuted); + } + } + + public static Builder newBuilder() { + return new Builder(); + } + + private final boolean balancerRan; + private final int movesCalculated; + private final int movesExecuted; + + private BalanceResponse(boolean balancerRan, int movesCalculated, int movesExecuted) { + this.balancerRan = balancerRan; + this.movesCalculated = movesCalculated; + this.movesExecuted = movesExecuted; + } + + /** + * Determines whether the balancer ran or not. The balancer may not run for a variety of reasons, Review comment: ```suggestion * Returns true if the balance ran, otherwise false. The balancer may not run for a variety of reasons, ``` ########## File path: hbase-client/src/main/java/org/apache/hadoop/hbase/client/BalanceResponse.java ########## @@ -0,0 +1,100 @@ +/* + * + * 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.client; + +import org.apache.yetus.audience.InterfaceAudience; +import org.apache.yetus.audience.InterfaceStability; + +/** + * Response returned from a balancer invocation + */ [email protected] [email protected] +public final class BalanceResponse { + + @InterfaceAudience.Public + @InterfaceStability.Evolving + public final static class Builder { + private boolean balancerRan; + private int movesCalculated; + private int movesExecuted; + + private Builder() {} + + public Builder setBalancerRan(boolean balancerRan) { + this.balancerRan = balancerRan; + return this; + } + + public Builder setMovesCalculated(int movesCalculated) { + this.movesCalculated = movesCalculated; + return this; + } + + public Builder setMovesExecuted(int movesExecuted) { + this.movesExecuted = movesExecuted; + return this; + } + + public BalanceResponse build() { + return new BalanceResponse(balancerRan, movesCalculated, movesExecuted); + } + } + + public static Builder newBuilder() { + return new Builder(); + } + + private final boolean balancerRan; + private final int movesCalculated; + private final int movesExecuted; + + private BalanceResponse(boolean balancerRan, int movesCalculated, int movesExecuted) { + this.balancerRan = balancerRan; + this.movesCalculated = movesCalculated; + this.movesExecuted = movesExecuted; + } + + /** + * Determines whether the balancer ran or not. The balancer may not run for a variety of reasons, + * such as: another balance is running, there are regions in transition, the cluster is in + * maintenance mode, etc. + */ + public boolean isBalancerRan() { + return balancerRan; + } + + /** + * The number of moves calculated by the balancer if it ran. This may be zero if + * no better balance could be found. + */ + public int getMovesCalculated() { + return movesCalculated; + } + + /** + * The number of moves actually executed by the balancer if it ran. This will be Review comment: `... if it ran` Would it help to express that this method is only valid if `isBalancerRan()` returns `true`? Then you can clearly state what this method returns (with the caveat that this method is only meaningful in some circumstances). -- 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]
