apurtell commented on a change in pull request #2261:
URL: https://github.com/apache/hbase/pull/2261#discussion_r477623990
##########
File path:
hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncAdmin.java
##########
@@ -1543,9 +1544,17 @@
* @param serverNames Server names to get slowlog responses from
* @param logQueryFilter filter to be used if provided
* @return Online slowlog response list. The return value wrapped by a
{@link CompletableFuture}
- */
- CompletableFuture<List<OnlineLogRecord>> getSlowLogResponses(final
Set<ServerName> serverNames,
- final LogQueryFilter logQueryFilter);
+ * @deprecated since 2.4.0 and will be removed in 4.0.0.
+ * Use {@link #getLogEntries(LogRequest, int)} instead.
+ */
+ default CompletableFuture<List<OnlineLogRecord>> getSlowLogResponses(
Review comment:
See above
##########
File path:
hbase-client/src/main/java/org/apache/hadoop/hbase/client/LogQueryFilter.java
##########
@@ -22,22 +22,27 @@
import org.apache.commons.lang3.builder.EqualsBuilder;
import org.apache.commons.lang3.builder.HashCodeBuilder;
import org.apache.commons.lang3.builder.ToStringBuilder;
+import org.apache.hadoop.hbase.ServerName;
import org.apache.yetus.audience.InterfaceAudience;
+import org.apache.yetus.audience.InterfaceStability;
+import java.util.Collections;
+import java.util.Set;
/**
* Slow/Large Log Query Filter with all filter and limit parameters
* Used by Admin API: getSlowLogResponses
*/
-@InterfaceAudience.Private
-public class LogQueryFilter {
+@InterfaceAudience.Public
+@InterfaceStability.Evolving
+public class LogQueryFilter extends LogRequest {
private String regionName;
private String clientAddress;
private String tableName;
private String userName;
- private int limit = 10;
Review comment:
If we can limit the slow log result set size with this filter that is
slow log specific, then we don't need a limit param on the existing admin API
for slow log. So keep this?
Apologies for the back and forth on this.
##########
File path: hbase-client/src/main/java/org/apache/hadoop/hbase/client/Admin.java
##########
@@ -2339,9 +2340,16 @@ boolean snapshotCleanupSwitch(final boolean on, final
boolean synchronous)
* @param logQueryFilter filter to be used if provided (determines slow /
large RPC logs)
* @return online slowlog response list
* @throws IOException if a remote or network exception occurs
+ * @deprecated since 2.4.0 and will be removed in 4.0.0.
+ * Use {@link #getLogEntries(LogRequest, int)} instead.
*/
- List<OnlineLogRecord> getSlowLogResponses(final Set<ServerName> serverNames,
- final LogQueryFilter logQueryFilter) throws IOException;
+ default List<OnlineLogRecord> getSlowLogResponses(final Set<ServerName>
serverNames,
Review comment:
Yes, this is what I meant by "add a method and leave this one for
backwards compat". Additional methods on an interface are binary compat per
Java spec and our compat guidelines as long as existing method signatures are
maintained.
If the LogQueryFilter can impose a limit, then that would work too.
##########
File path:
hbase-client/src/main/java/org/apache/hadoop/hbase/client/RawAsyncHBaseAdmin.java
##########
@@ -4212,4 +4213,29 @@ private void getProcedureResult(long procId,
CompletableFuture<Void> future, int
(s, c, req, done) -> s.updateRSGroupConfig(c, req, done), resp
-> null))
).call();
}
+
+ private CompletableFuture<List<LogEntry>> getBalancerDecisions(
Review comment:
MasterProtos.BalancerDecisionsRequest is not generic. This is the issue.
We should be using one common generic RPC API to get slow log and balancer ring
entries, and any other named ring entry type. This uses a new special master
RPC for balancer decision.
##########
File path: hbase-protocol-shaded/src/main/protobuf/server/master/Master.proto
##########
@@ -693,6 +694,14 @@ message SwitchExceedThrottleQuotaResponse {
required bool previous_exceed_throttle_quota_enabled = 1;
}
+message BalancerDecisionsRequest {
Review comment:
Not impossible at all.
getRingEntries RPC in RS RPC servcies
getRingEntries RPC in master RPC services
> we can't achieve abstraction since protobuf doesn't support is-a
relationship among proto messages.
You have an outer message type that is a class name and a byte string, and
the inner protobuf is stuffed into the byte string while the class name is set
to the type you need to use to deserialize.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
