Re: [PR] Change the policy of the queue(DataCarrier) L1 in the metric aggregate worker to IF_POSSIBLE mode [skywalking]

["kezhenxu94 (via GitHub)" <gi...@apache.org>]

kezhenxu94 commented on code in PR #11554:
URL: https://github.com/apache/skywalking/pull/11554#discussion_r1395437537


##########
oap-server/server-library/library-datacarrier-queue/src/main/java/org/apache/skywalking/oap/server/library/datacarrier/buffer/ArrayBlockingQueueBuffer.java:
##########
@@ -40,8 +40,10 @@ public class ArrayBlockingQueueBuffer<T> implements QueueBuffer<T> {
 
     @Override
     public boolean save(T data) {
-        //only BufferStrategy.BLOCKING
         try {
+            if (BufferStrategy.IF_POSSIBLE.equals(strategy) && queue.remainingCapacity() == 0) {
+                return false;
+            }

Review Comment:
   From the javadoc of `queue.remainingCapacity`, this is a misuse
   
   ```
        * <p>Note that you <em>cannot</em> always tell if an attempt to insert
        * an element will succeed by inspecting {@code remainingCapacity}
        * because it may be the case that another thread is about to
        * insert or remove an element.
   ```
   
   And I'm curious why don't we remove `ArrayBlockingQueueBuffer.java` and keep `Buffer.java`, I personally think the `Buffer` class is more easy to read.



-- 
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: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org