bharathv commented on a change in pull request #754: HBASE-22978 : Online slow response log URL: https://github.com/apache/hbase/pull/754#discussion_r379951556
########## File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/slowlog/SlowLogEventHandler.java ########## @@ -0,0 +1,154 @@ +/* + * + * 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.regionserver.slowlog; + +import com.lmax.disruptor.EventHandler; +import com.lmax.disruptor.RingBuffer; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; +import java.util.Queue; +import java.util.stream.Collectors; + +import org.apache.commons.lang3.StringUtils; +import org.apache.yetus.audience.InterfaceAudience; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import org.apache.hbase.thirdparty.com.google.common.collect.EvictingQueue; +import org.apache.hbase.thirdparty.com.google.common.collect.Queues; + +import org.apache.hadoop.hbase.shaded.protobuf.generated.AdminProtos; +import org.apache.hadoop.hbase.shaded.protobuf.generated.TooSlowLog.SlowLogPayload; + +/** + * Event Handler run by disruptor ringbuffer consumer + */ +@InterfaceAudience.Private +class SlowLogEventHandler implements EventHandler<RingBufferEnvelope> { + + private static final Logger LOG = LoggerFactory.getLogger(SlowLogEventHandler.class); + + private final Queue<SlowLogPayload> queue; + + SlowLogEventHandler(int eventCount) { + EvictingQueue<SlowLogPayload> evictingQueue = EvictingQueue.create(eventCount); Review comment: I think you misunderstood my comment.. My point was to get rid of lmax's ringbuffer implementation with and asynchronous thread-safe EvictingQueue (it isn't as complex as it sounds)... When I meant asynchronous, any entries added by the event producer (RpcServer here) is asynchronously processed by a single thread. This is easier to reason about than CompletableFuture.runAsync() approach you did on the caller. To recap my original comment, the implementation should be something like this I think. - We have a shared blocking queue between the producer (RpcServer) and consumer (the thread that serializes the input and puts it in an evicting queue). All the producer does is put events (too long/too big) events in this shared queue which automatically makes the whole thing async from a producer POV. - The consumer (a single thread), keeps polling entries, serializes them (also does logging) and puts them into a ring buffer This would be much fewer lines of code and much more simple to reason about than using the lmax's ring buffer (with very similar performance for our use case). Thoughts? ---------------------------------------------------------------- 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 With regards, Apache Git Services
