virajjasani commented on a change in pull request #2261:
URL: https://github.com/apache/hbase/pull/2261#discussion_r478097593
##########
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:
> If the LogQueryFilter can impose a limit, then that would work too.
You mean to say the previous approach where individual request payloads had
limit themselves (LogQueryFilter and BalancerDecisionRequest both have limit as
a param) was good enough? Or you are suggesting to just let LogQueryFilter
(which represents slowLog use-case) have limit param?
I somehow still feel, instead of exposing `limit` separately on Admin API,
we should let individual request payloads have `limit` (like previous commit)
##########
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
*/
[email protected]
-public class LogQueryFilter {
[email protected]
[email protected]
+public class LogQueryFilter extends LogRequest {
private String regionName;
private String clientAddress;
private String tableName;
private String userName;
- private int limit = 10;
Review comment:
Yeah exactly, I am also more inclined to keep `limit` in request
payloads. Instead, let me keep `limit` in abstract payload `LogRequest` which
is empty right now(IS.evolving) and also going to be extended by every new
use-case's own request payloads.
##########
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:
> You have an outer message type that is a class name and a byte string,
and the protobuf for the inner type is serialized into the byte string while
the class name field of the outer type is set to the type to use for parsing
the inner type.
Thanks a lot, I was not aware of this usage. Yes, passing an object as
ByteString should be good enough.
----------------------------------------------------------------
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:
[email protected]
