bbeaudreault commented on code in PR #4937:
URL: https://github.com/apache/hbase/pull/4937#discussion_r1082947181
##########
hbase-client/src/main/java/org/apache/hadoop/hbase/client/OnlineLogRecord.java:
##########
@@ -128,11 +193,52 @@ public int getMultiServiceCalls() {
return multiServiceCalls;
}
+ /**
+ * If {@value
org.apache.hadoop.hbase.HConstants#SLOW_LOG_OPERATION_MESSAGE_PAYLOAD_ENABLED}
is
+ * enabled then this value may be present and should represent the Scan that
produced the given
+ * {@link OnlineLogRecord}. This value should only be present if {@link
#getMulti()},
+ * {@link #getGet()}, and {@link #getMutate()} are empty
+ */
+ public Optional<Scan> getScan() {
+ return scan;
+ }
+
+ /**
+ * If {@value
org.apache.hadoop.hbase.HConstants#SLOW_LOG_OPERATION_MESSAGE_PAYLOAD_ENABLED}
is
+ * enabled then this value may be present and should represent the
MultiRequest that produced the
+ * given {@link OnlineLogRecord}. This value should only be present if
{@link #getScan},
+ * {@link #getGet()}, and {@link #getMutate()} are empty
+ */
+ public Optional<List<Operation>> getMulti() {
+ return multi;
+ }
+
+ /**
+ * If {@value
org.apache.hadoop.hbase.HConstants#SLOW_LOG_OPERATION_MESSAGE_PAYLOAD_ENABLED}
is
+ * enabled then this value may be present and should represent the Get that
produced the given
+ * {@link OnlineLogRecord}. This value should only be present if {@link
#getScan()},
+ * {@link #getMulti()} ()}, and {@link #getMutate()} are empty
+ */
+ public Optional<Get> getGet() {
+ return get;
+ }
+
+ /**
+ * If {@value
org.apache.hadoop.hbase.HConstants#SLOW_LOG_OPERATION_MESSAGE_PAYLOAD_ENABLED}
is
+ * enabled then this value may be present and should represent the Mutation
that produced the
+ * given {@link OnlineLogRecord}. This value should only be present if
{@link #getScan},
+ * {@link #getMulti()} ()}, and {@link #getGet()} ()} are empty
+ */
+ public Optional<Mutation> getMutate() {
+ return mutate;
+ }
Review Comment:
We should think a little about whether this is actually the API we want to
expose. Since this class is InterfaceAudience.Public, any changes we make here
we're stuck with for a long time. We can only remove/rename methods (i.e.
breaking changes) at major version releases and we haven't had a major version
release in years.
There's potentially a little more flexibility since it's
InterfaceStability.Evolving, but I don't think there's really consensus across
all committers/PMC as to whether this grants us anything. It's not mentioned in
our guides except:
> Public packages marked as evolving may be changed, but it is discouraged.
Anyway, not saying what we have here is wrong per-se. But we should take a
minute to think about how we might evolve usage of these slow log stuff over
time and make sure what we have here will support that.
--
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]
