From f0a3e933b91c1e5ec964e44073643b7bb8cc5e50 Mon Sep 17 00:00:00 2001 From: EvanMi <43713766+EvanMi@users.noreply.github.com> Date: Wed, 20 Dec 2023 14:34:53 +0800 Subject: [PATCH] [ISSUE #7684] Fix iterator.remove() bug (#7682) * bugfix: CopyOnWriteArray#listIterator do not support remove action when iterating * add testcase --------- Co-authored-by: mipengcheng3 --- .../apache/rocketmq/store/MappedFileQueue.java | 8 +++++++- .../rocketmq/store/MappedFileQueueTest.java | 15 +++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/store/src/main/java/org/apache/rocketmq/store/MappedFileQueue.java b/store/src/main/java/org/apache/rocketmq/store/MappedFileQueue.java index 9a0824829e..e32c16a82a 100644 --- a/store/src/main/java/org/apache/rocketmq/store/MappedFileQueue.java +++ b/store/src/main/java/org/apache/rocketmq/store/MappedFileQueue.java @@ -406,6 +406,7 @@ public class MappedFileQueue implements Swappable { } ListIterator iterator = this.mappedFiles.listIterator(mappedFiles.size()); + List toRemoves = new ArrayList<>(); while (iterator.hasPrevious()) { mappedFileLast = iterator.previous(); @@ -416,9 +417,14 @@ public class MappedFileQueue implements Swappable { mappedFileLast.setCommittedPosition(where); break; } else { - iterator.remove(); + toRemoves.add(mappedFileLast); } } + + if (!toRemoves.isEmpty()) { + this.mappedFiles.removeAll(toRemoves); + } + return true; } diff --git a/store/src/test/java/org/apache/rocketmq/store/MappedFileQueueTest.java b/store/src/test/java/org/apache/rocketmq/store/MappedFileQueueTest.java index d92b3cbc0d..3cc17c659b 100644 --- a/store/src/test/java/org/apache/rocketmq/store/MappedFileQueueTest.java +++ b/store/src/test/java/org/apache/rocketmq/store/MappedFileQueueTest.java @@ -477,6 +477,21 @@ public class MappedFileQueueTest { TimeUnit.SECONDS.sleep(3); } + @Test + public void testReset() { + final String fixedMsg = "0123456789abcdef"; + MappedFileQueue mappedFileQueue = + new MappedFileQueue(storePath + File.separator + "a/", 64, null); + for (int i = 0; i < 8; i++) { + MappedFile mappedFile = mappedFileQueue.getLastMappedFile(0); + assertThat(mappedFile).isNotNull(); + assertThat(mappedFile.appendMessage(fixedMsg.getBytes())).isTrue(); + } + assertThat(mappedFileQueue.getMappedFiles().size()).isEqualTo(2); + assertThat(mappedFileQueue.resetOffset(0)).isTrue(); + assertThat(mappedFileQueue.getMappedFiles().size()).isEqualTo(1); + } + @After public void destroy() { File file = new File(storePath);