rpuch commented on code in PR #6237:
URL: https://github.com/apache/ignite-3/pull/6237#discussion_r2200795554
##
modules/raft/src/main/java/org/apache/ignite/raft/jraft/option/NodeOptions.java:
##
@@ -301,6 +303,14 @@ public class NodeOptions extends RpcOptions implements
Copiable {
*/
private boolean isSystemGroup = false;
+/**
+ * Shared pool of {@link ByteBufferCollector} for sending log entries for
replication.
+ *
+ * Used to prevent a large number of {@link ByteBufferCollector} from
being accumulated across all threads that are involved in
+ * sending log entries, see {@link
ByteBufferCollector#allocateByRecyclers}.
+ */
+private ByteBufferCollectorPool appendEntriesByteBufferCollectorPool;
Review Comment:
It seems that in a few places in our code, the same implementation is
assigned for this option. Would it make sense to initialize the field with it
right here? In this way we'll make it more difficult to have different pools in
different tests, for example
##
modules/raft/src/main/java/org/apache/ignite/raft/jraft/util/concurrent/ConcurrentLinkedFifoByteBufferCollectorPool.java:
##
@@ -0,0 +1,88 @@
+/*
+ * 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.ignite.raft.jraft.util.concurrent;
+
+import java.util.concurrent.atomic.AtomicReference;
+import org.apache.ignite.raft.jraft.util.ByteBufferCollector;
+import org.apache.ignite.raft.jraft.util.ByteBufferCollectorPool;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Thread safe implementation of a pool based on a fixed-size FIFO linked list.
+ *
+ * {@link ByteBufferCollector} are given out on a first-in, first-out
basis.
+ */
+public class ConcurrentLinkedFifoByteBufferCollectorPool implements
ByteBufferCollectorPool {
+private final int capacity;
+
+private final AtomicReference stack = new AtomicReference<>();
+
+/**
+ * Constructor.
+ *
+ * @param capacity Maximum capacity of the pool, expected to be greater
than zero.
+ */
+public ConcurrentLinkedFifoByteBufferCollectorPool(int capacity) {
+assert capacity > 0 : capacity;
+
+this.capacity = capacity;
+}
+
+/** Removes and returns the last added {@link ByteBufferCollector}, {@code
null} if the pool is empty. */
+@Override
Review Comment:
Do we really need javadocs on overrides?
##
modules/raft/src/test/java/org/apache/ignite/raft/jraft/util/concurrent/ConcurrentLinkedLifoByteBufferCollectorPoolTest.java:
##
@@ -0,0 +1,191 @@
+/*
+ * 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.ignite.raft.jraft.util.concurrent;
+
+import static java.util.stream.Collectors.toList;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.hasItems;
+import static org.hamcrest.Matchers.hasSize;
+import static org.hamcrest.Matchers.not;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertSame;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.stream.IntStream;
+import org.apache.ignite.internal.testframework.IgniteTestUtils;
+import org.apache.ignite.raft.jraft.util.ByteBufferCollector;
+import org.junit.jupiter.api.Re