Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
206 changes: 206 additions & 0 deletions drmq-broker/src/main/java/com/drmq/broker/AdminHttpServer.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,206 @@
package com.drmq.broker;

import com.google.gson.Gson;
import com.google.gson.JsonArray;
import com.google.gson.JsonObject;
import com.sun.net.httpserver.HttpExchange;
import com.sun.net.httpserver.HttpHandler;
import com.sun.net.httpserver.HttpServer;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import java.io.IOException;
import java.io.OutputStream;
import java.net.InetSocketAddress;
import java.util.List;

/**
* Lightweight HTTP server for administrative REST APIs.
*/
public class AdminHttpServer {
private static final Logger logger = LoggerFactory.getLogger(AdminHttpServer.class);
private final HttpServer server;
private final MessageStore messageStore;
private final OffsetManager offsetManager;
private final ConsumerGroupCoordinator groupCoordinator;
private final Gson gson = new Gson();

public AdminHttpServer(int port, MessageStore messageStore, OffsetManager offsetManager, ConsumerGroupCoordinator groupCoordinator) throws IOException {
this.messageStore = messageStore;
this.offsetManager = offsetManager;
this.groupCoordinator = groupCoordinator;

this.server = HttpServer.create(new InetSocketAddress(port), 0);

this.server.createContext("/api/topics", this::handleTopics);
this.server.createContext("/api/consumers", this::handleConsumers);
this.server.createContext("/api/messages", this::handleMessages);

// CORS and standard executor
this.server.setExecutor(null);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

sed -n '1,220p' drmq-broker/src/main/java/com/drmq/broker/AdminHttpServer.java

Repository: samuel025/DRMQ

Length of output: 8486


🌐 Web query:

JDK HttpServer.setExecutor(null) default executor single threaded behavior

💡 Result:

When you call HttpServer.setExecutor(null) (or fail to set an executor before starting the server), the JDK HttpServer uses a default executor implementation [1][2][3]. This default executor is synchronous and single-threaded [4][5]. Internally, the server uses a static class called DefaultExecutor, which executes tasks by calling task.run directly within the thread that manages the request dispatching [4][5]. Consequently, all HTTP requests are handled serially in the same thread, meaning they are processed in a blocking manner [5]. As a result, the server cannot process multiple requests in parallel when using the default executor [5]. To enable parallel request processing, you must provide a custom Executor, such as one returned by java.util.concurrent.Executors (e.g., Executors.newCachedThreadPool or Executors.newFixedThreadPool) [5].

Citations:


Use a dedicated executor for the admin server. setExecutor(null) leaves requests on the JDK default executor, so one slow /api/messages call can block /api/topics and /api/consumers. Use a small dedicated pool here to keep the dashboard responsive.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@drmq-broker/src/main/java/com/drmq/broker/AdminHttpServer.java` at line 40,
Replace the null executor configured in AdminHttpServer with a small dedicated
executor for the admin HTTP server, using the server’s setExecutor call. Ensure
the pool is suitable for handling concurrent dashboard endpoints so slow
/api/messages requests do not block /api/topics or /api/consumers.

}
Comment on lines +20 to +41

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
rg -n --type=java -C2 'Authorization|BasicAuth|ApiKey|apiKey|Authenticat' drmq-broker/src/main/java/com/drmq/broker

Repository: samuel025/DRMQ

Length of output: 784


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '--- AdminHttpServer.java outline ---'
ast-grep outline drmq-broker/src/main/java/com/drmq/broker/AdminHttpServer.java --view expanded || true

echo '--- Relevant AdminHttpServer.java slices ---'
sed -n '1,260p' drmq-broker/src/main/java/com/drmq/broker/AdminHttpServer.java

echo '--- Search for admin server construction / port binding / auth config ---'
rg -n --type=java -C2 'new AdminHttpServer|admin.*port|adminPort|HttpServer.create|setExecutor\(|Access-Control-Allow-Origin|Authorization|Basic|auth' drmq-broker/src/main/java

Repository: samuel025/DRMQ

Length of output: 12774


Lock down the admin API
HttpServer.create(new InetSocketAddress(port), 0) binds to all interfaces, and the handlers also return Access-Control-Allow-Origin: * with no auth check. That makes /api/messages and /api/consumers readable by any reachable client, including raw payloads and consumer offsets. Bind to loopback by default and add authentication before exposing this outside local development.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@drmq-broker/src/main/java/com/drmq/broker/AdminHttpServer.java` around lines
20 - 41, Update the AdminHttpServer constructor and request-handling flow to
bind HttpServer to loopback instead of all interfaces by default, and require
authentication before serving /api/topics, /api/consumers, or /api/messages.
Replace the wildcard CORS behavior with an authenticated, restricted origin
policy, using the existing handlers such as handleTopics, handleConsumers, and
handleMessages without exposing data to unauthenticated clients.


public void start() {
server.start();
logger.info("Admin HTTP Server started on port {}", server.getAddress().getPort());
}

public void stop() {
server.stop(1);
logger.info("Admin HTTP Server stopped.");
}

private void handleTopics(HttpExchange exchange) throws IOException {
addCorsHeaders(exchange);
if ("OPTIONS".equals(exchange.getRequestMethod())) {
exchange.sendResponseHeaders(204, -1);
return;
}

JsonArray topicsArray = new JsonArray();
List<String> topics = messageStore.getTopics();

for (String topic : topics) {
JsonObject obj = new JsonObject();
obj.addProperty("name", topic);
obj.addProperty("messageCount", messageStore.getMessageCount(topic));
// The global offset is global, not per topic.
// We'll just return the message count for now.
topicsArray.add(obj);
}

sendJsonResponse(exchange, 200, gson.toJson(topicsArray));
}

private void handleConsumers(HttpExchange exchange) throws IOException {
addCorsHeaders(exchange);
if ("OPTIONS".equals(exchange.getRequestMethod())) {
exchange.sendResponseHeaders(204, -1);
return;
}

JsonArray groupsArray = new JsonArray();
java.util.Map<String, Long> allOffsets = offsetManager.getAllOffsets();

// Group by consumer group name
java.util.Map<String, JsonArray> groupsMap = new java.util.HashMap<>();

for (java.util.Map.Entry<String, Long> entry : allOffsets.entrySet()) {
String[] parts = entry.getKey().split("/");
if (parts.length != 2) continue;
Comment on lines +88 to +90

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

split("/") silently drops any consumer/topic entry whose topic name contains "/".

OffsetManager.key() joins as group + "/" + topic. If topic itself contains a "/", split("/") yields more than 2 parts and the if (parts.length != 2) continue; guard silently skips that group/topic pair from the consumer-lag view instead of parsing it correctly.

🐛 Proposed fix: limit split to 2 parts
-            String[] parts = entry.getKey().split("/");
+            String[] parts = entry.getKey().split("/", 2);
Note this only fixes slash-containing topic names; a slash-containing consumer group name would still be mis-parsed, which is a pre-existing ambiguity in the `"group/topic"` key scheme.
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
for (java.util.Map.Entry<String, Long> entry : allOffsets.entrySet()) {
String[] parts = entry.getKey().split("/");
if (parts.length != 2) continue;
for (java.util.Map.Entry<String, Long> entry : allOffsets.entrySet()) {
String[] parts = entry.getKey().split("/", 2);
if (parts.length != 2) continue;
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@drmq-broker/src/main/java/com/drmq/broker/AdminHttpServer.java` around lines
88 - 90, Update the key parsing in the allOffsets iteration to split the entry
key at only the first "/" so topics may contain additional slashes. Keep the
existing two-part validation and consumer-lag handling, while preserving the
known group-name ambiguity in the "group/topic" key format.


String groupName = parts[0];
String topicName = parts[1];
long committedOffset = entry.getValue();

// Calculate lag using true topic head offset rather than message count
// Since DRMQ uses global offsets, messageCount does not correlate to the offset values.
long headOffset = messageStore.getHeadOffset(topicName);
long lag = 0;
if (headOffset >= 0) {
long effectiveCommitted = Math.max(0, committedOffset); // -1 means none committed
lag = Math.max(0, (headOffset + 1) - effectiveCommitted);
}

JsonObject topicObj = new JsonObject();
topicObj.addProperty("topic", topicName);
topicObj.addProperty("headOffset", headOffset);
topicObj.addProperty("committedOffset", committedOffset);
topicObj.addProperty("lag", lag);
topicObj.addProperty("activeMembers", groupCoordinator.getConsumerCount(groupName, topicName));

groupsMap.computeIfAbsent(groupName, k -> new JsonArray()).add(topicObj);
}

for (java.util.Map.Entry<String, JsonArray> entry : groupsMap.entrySet()) {
JsonObject groupObj = new JsonObject();
groupObj.addProperty("groupId", entry.getKey());
groupObj.add("topics", entry.getValue());
groupsArray.add(groupObj);
}

sendJsonResponse(exchange, 200, gson.toJson(groupsArray));
}

private void handleMessages(HttpExchange exchange) throws IOException {
addCorsHeaders(exchange);
if ("OPTIONS".equals(exchange.getRequestMethod())) {
exchange.sendResponseHeaders(204, -1);
return;
}

try {
String query = exchange.getRequestURI().getQuery();
if (query == null) {
sendJsonResponse(exchange, 400, "{\"error\":\"Missing query parameters\"}");
return;
}

String topic = null;
long offset = 0;
int limit = 10;
Long timestamp = null;

for (String param : query.split("&")) {
String[] pair = param.split("=");
if (pair.length == 2) {
if ("topic".equals(pair[0])) topic = pair[1];
else if ("offset".equals(pair[0])) offset = Long.parseLong(pair[1]);
else if ("limit".equals(pair[0])) limit = Integer.parseInt(pair[1]);
else if ("timestamp".equals(pair[0])) timestamp = Long.parseLong(pair[1]);
}
}

if (topic == null) {
sendJsonResponse(exchange, 400, "{\"error\":\"Missing 'topic' parameter\"}");
return;
}

// Limit bounds to avoid OOM
limit = Math.min(100, Math.max(1, limit));

if (timestamp != null && timestamp > 0) {
offset = messageStore.findOffsetByTimestamp(topic, timestamp);
if (offset == -1) {
sendJsonResponse(exchange, 200, "[]");
return;
}
}

List<com.drmq.protocol.DRMQProtocol.StoredMessage> messages = messageStore.getMessages(topic, offset, limit);
JsonArray msgsArray = new JsonArray();

for (com.drmq.protocol.DRMQProtocol.StoredMessage msg : messages) {
JsonObject obj = new JsonObject();
obj.addProperty("offset", msg.getOffset());
obj.addProperty("timestamp", msg.getTimestamp());
obj.addProperty("storedAt", msg.getStoredAt());
if (msg.hasKey()) {
obj.addProperty("key", msg.getKey());
}
obj.addProperty("payload", msg.getPayload().toStringUtf8());
msgsArray.add(obj);
}

sendJsonResponse(exchange, 200, gson.toJson(msgsArray));
} catch (Exception e) {
logger.error("Error handling messages request", e);
sendJsonResponse(exchange, 500, "{\"error\":\"" + e.getMessage() + "\"}");
}
Comment on lines +186 to +189

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Unescaped exception message can produce malformed JSON error responses.

e.getMessage() is concatenated directly into a JSON string literal. NumberFormatException messages (a likely cause here, from Long.parseLong/Integer.parseInt on bad query params) contain embedded double quotes (e.g. For input string: "abc"), which breaks the resulting JSON and defeats Topics.tsx's data.error handling.

🐛 Proposed fix using Gson to build the error object
         } catch (Exception e) {
             logger.error("Error handling messages request", e);
-            sendJsonResponse(exchange, 500, "{\"error\":\"" + e.getMessage() + "\"}");
+            JsonObject err = new JsonObject();
+            err.addProperty("error", e.getMessage());
+            sendJsonResponse(exchange, 500, gson.toJson(err));
         }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
} catch (Exception e) {
logger.error("Error handling messages request", e);
sendJsonResponse(exchange, 500, "{\"error\":\"" + e.getMessage() + "\"}");
}
} catch (Exception e) {
logger.error("Error handling messages request", e);
JsonObject err = new JsonObject();
err.addProperty("error", e.getMessage());
sendJsonResponse(exchange, 500, gson.toJson(err));
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@drmq-broker/src/main/java/com/drmq/broker/AdminHttpServer.java` around lines
186 - 189, Update the exception response in AdminHttpServer’s message-request
handler to construct the error object through the existing JSON serializer (such
as Gson) instead of concatenating e.getMessage() into a JSON string. Ensure
messages containing quotes or other special characters remain valid JSON so
Topics.tsx can read data.error.

}

private void addCorsHeaders(HttpExchange exchange) {
exchange.getResponseHeaders().add("Access-Control-Allow-Origin", "*");
exchange.getResponseHeaders().add("Access-Control-Allow-Methods", "GET, POST, DELETE, OPTIONS");
exchange.getResponseHeaders().add("Access-Control-Allow-Headers", "Content-Type, Authorization");
}

private void sendJsonResponse(HttpExchange exchange, int statusCode, String response) throws IOException {
byte[] bytes = response.getBytes("UTF-8");
exchange.getResponseHeaders().add("Content-Type", "application/json");
exchange.sendResponseHeaders(statusCode, bytes.length);
try (OutputStream os = exchange.getResponseBody()) {
os.write(bytes);
}
}
}
8 changes: 8 additions & 0 deletions drmq-broker/src/main/java/com/drmq/broker/BrokerServer.java
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ public class BrokerServer {
private final List<RaftPeer> raftPeers;
private final BrokerMetrics metrics;
private TelemetryWebSocketServer telemetryServer;
private AdminHttpServer adminHttpServer;

private volatile boolean running = false;

Expand Down Expand Up @@ -171,6 +172,10 @@ public void initChannel(SocketChannel ch) {
telemetryServer = new TelemetryWebSocketServer(wsPort, this);
telemetryServer.start();

int adminPort = config.getPort() + 300;
adminHttpServer = new AdminHttpServer(adminPort, messageStore, offsetManager, groupCoordinator);
adminHttpServer.start();

logger.info("DRMQ Broker started on port {} with data directory {}",
config.getPort(), config.getDataDir());

Expand Down Expand Up @@ -214,6 +219,9 @@ public void shutdown() {
if (telemetryServer != null) {
telemetryServer.shutdown();
}
if (adminHttpServer != null) {
adminHttpServer.stop();
}

if (activeChannels != null) {
activeChannels.close().awaitUninterruptibly();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -293,7 +293,6 @@ public int getActiveLeasesCount() {
return total;
}

// ---- Dead-Letter Queue (DLQ) Support ----

/**
* Explicitly reject (NACK) a message offset for a consumer within a group.
Expand All @@ -319,7 +318,6 @@ public boolean nackOffset(String group, String topic, String consumerId, long of

state.lock.lock();
try {
// Remove the consumer's active lease
Lease lease = state.activeLeases.remove(consumerId);
if (lease != null) {
state.members.remove(consumerId);
Expand All @@ -333,7 +331,6 @@ public boolean nackOffset(String group, String topic, String consumerId, long of
routeToDlq(state, group, topic, offset);
return true;
} else {
// Rewind for redelivery using the lease's fromOffset to not skip messages
long rewindOffset = (lease != null) ? lease.fromOffset : offset;
if (rewindOffset < state.dispatchOffset) {
state.dispatchOffset = rewindOffset;
Expand Down Expand Up @@ -379,15 +376,13 @@ private void routeToDlq(GroupTopicState state, String group, String topic, long
}
});

// Advance past the bad offset regardless — don't let a DLQ write failure block progress
long nextOffset = badOffset + 1;
state.committedRanges.add(new CommittedRange(badOffset, nextOffset));
advanceCommittedOffset(state, group, topic);
if (state.dispatchOffset <= badOffset) {
state.dispatchOffset = nextOffset;
}

// Clean up the delivery counter for this offset
state.deliveryCounts.remove(badOffset);
}

Expand Down
33 changes: 32 additions & 1 deletion drmq-broker/src/main/java/com/drmq/broker/MessageStore.java
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,9 @@ public class MessageStore implements Closeable {

// Topic -> Total number of messages
private final ConcurrentHashMap<String, AtomicLong> topicMessageCounts = new ConcurrentHashMap<>();

// Topic -> Highest offset appended
private final ConcurrentHashMap<String, AtomicLong> topicHeadOffsets = new ConcurrentHashMap<>();

// In-memory cache for recent messages (Topic -> BoundedMessageCache)
private final ConcurrentHashMap<String, BoundedMessageCache> messageCache = new ConcurrentHashMap<>();
Expand Down Expand Up @@ -124,6 +127,11 @@ private void recoverInternal() throws IOException {
addToCache(topic, message);
topicMessageCounts.computeIfAbsent(topic, k -> new AtomicLong()).incrementAndGet();

AtomicLong head = topicHeadOffsets.computeIfAbsent(topic, k -> new AtomicLong(-1));
if (offset > head.get()) {
head.set(offset);
}

if (offset > maxOffset) {
maxOffset = offset;
}
Expand Down Expand Up @@ -156,6 +164,7 @@ public void reload() throws IOException {
logger.info("Reloading MessageStore state from disk...");
topicIndex.clear();
topicMessageCounts.clear();
topicHeadOffsets.clear();
messageCache.clear();
topicWriteLocks.clear();

Expand Down Expand Up @@ -222,6 +231,11 @@ public long append(String topic, byte[] payload, String key, long clientTimestam

addToCache(topic, message);
topicMessageCounts.computeIfAbsent(topic, k -> new AtomicLong()).incrementAndGet();

AtomicLong head = topicHeadOffsets.computeIfAbsent(topic, k -> new AtomicLong(-1));
if (offset > head.get()) {
head.set(offset);
}
Comment on lines +234 to +238

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Non-atomic "set-if-greater" on topicHeadOffsets races under concurrent appends to the same topic.

head.get()/head.set() here runs outside the synchronized(topicLock) block, only under the shared globalLock.readLock(). Two threads appending to the same topic concurrently can interleave the check-then-set and leave head at a stale, lower value than the true max offset — this directly feeds getHeadOffset(), which AdminHttpServer.handleConsumers uses to compute consumer lag.

🔒 Proposed fix using atomic max update
-            AtomicLong head = topicHeadOffsets.computeIfAbsent(topic, k -> new AtomicLong(-1));
-            if (offset > head.get()) {
-                head.set(offset);
-            }
+            topicHeadOffsets.computeIfAbsent(topic, k -> new AtomicLong(-1))
+                    .accumulateAndGet(offset, Math::max);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
AtomicLong head = topicHeadOffsets.computeIfAbsent(topic, k -> new AtomicLong(-1));
if (offset > head.get()) {
head.set(offset);
}
topicHeadOffsets.computeIfAbsent(topic, k -> new AtomicLong(-1))
.accumulateAndGet(offset, Math::max);
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@drmq-broker/src/main/java/com/drmq/broker/MessageStore.java` around lines 234
- 238, Make the update to topicHeadOffsets in the append path atomic: replace
the separate head.get()/head.set() comparison with an AtomicLong update
operation that retains the maximum of the current value and offset. Keep the
existing computeIfAbsent initialization and topic-scoped behavior unchanged.


logger.debug("Persisted and indexed message: topic={}, offset={}, position={}, segment={}",
topic, offset, position, segment.getFilePath().getFileName());
Expand Down Expand Up @@ -292,11 +306,15 @@ public long appendBatch(String topic, List<ProduceBatchRequest.BatchEntry> entri
}

AtomicLong counter = topicMessageCounts.computeIfAbsent(topic, k -> new AtomicLong());
AtomicLong head = topicHeadOffsets.computeIfAbsent(topic, k -> new AtomicLong(-1));
for (int i = 0; i < messages.size(); i++) {
StoredMessage msg = messages.get(i);
indexMessage(topic, msg.getOffset(), positions.get(i));
addToCache(topic, msg);
counter.incrementAndGet();
if (msg.getOffset() > head.get()) {
head.set(msg.getOffset());
}
Comment on lines +309 to +317

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Same non-atomic "set-if-greater" race as in append(), here in the batch loop.

Same TOCTOU hazard: head.get()/head.set() is not atomic and can race with a concurrent append()/appendBatch() on the same topic, causing getHeadOffset() to under-report the true head.

🔒 Proposed fix using atomic max update
-            AtomicLong head = topicHeadOffsets.computeIfAbsent(topic, k -> new AtomicLong(-1));
-            for (int i = 0; i < messages.size(); i++) {
-                StoredMessage msg = messages.get(i);
-                indexMessage(topic, msg.getOffset(), positions.get(i));
-                addToCache(topic, msg);
-                counter.incrementAndGet();
-                if (msg.getOffset() > head.get()) {
-                    head.set(msg.getOffset());
-                }
-            }
+            AtomicLong head = topicHeadOffsets.computeIfAbsent(topic, k -> new AtomicLong(-1));
+            for (int i = 0; i < messages.size(); i++) {
+                StoredMessage msg = messages.get(i);
+                indexMessage(topic, msg.getOffset(), positions.get(i));
+                addToCache(topic, msg);
+                counter.incrementAndGet();
+            }
+            head.accumulateAndGet(currentOffset - 1, Math::max);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
AtomicLong head = topicHeadOffsets.computeIfAbsent(topic, k -> new AtomicLong(-1));
for (int i = 0; i < messages.size(); i++) {
StoredMessage msg = messages.get(i);
indexMessage(topic, msg.getOffset(), positions.get(i));
addToCache(topic, msg);
counter.incrementAndGet();
if (msg.getOffset() > head.get()) {
head.set(msg.getOffset());
}
AtomicLong head = topicHeadOffsets.computeIfAbsent(topic, k -> new AtomicLong(-1));
for (int i = 0; i < messages.size(); i++) {
StoredMessage msg = messages.get(i);
indexMessage(topic, msg.getOffset(), positions.get(i));
addToCache(topic, msg);
counter.incrementAndGet();
}
head.accumulateAndGet(currentOffset - 1, Math::max);
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@drmq-broker/src/main/java/com/drmq/broker/MessageStore.java` around lines 309
- 317, Update the head-offset update inside the batch loop in MessageStore so
the comparison and update use an atomic max operation on the per-topic
AtomicLong, matching the concurrency-safe behavior required by append().
Preserve the existing indexing, caching, and counter updates.

}

logger.debug("Batch persisted: topic={}, baseOffset={}, count={}", topic, baseOffset, batchSize);
Expand Down Expand Up @@ -523,6 +541,11 @@ public int getMessageCount(String topic) {
return count == null ? 0 : count.intValue();
}

public long getHeadOffset(String topic) {
AtomicLong head = topicHeadOffsets.get(topic);
return head == null ? -1 : head.get();
}

public List<String> getTopics() {
return new ArrayList<>(topicMessageCounts.keySet());
}
Expand All @@ -542,6 +565,7 @@ public long getCachedMessageCount() {
public void clear() {
topicIndex.clear();
topicMessageCounts.clear();
topicHeadOffsets.clear();
messageCache.clear();
globalOffset.set(0);
logger.info("Message store memory state cleared");
Expand Down Expand Up @@ -682,9 +706,16 @@ public void removeRange(long fromOffset, long toOffset) {
public List<StoredMessage> getMessagesFrom(long fromOffset, int maxCount) {
lock.readLock().lock();
try {
if (!cache.containsKey(fromOffset)) {
return Collections.emptyList();
}
List<StoredMessage> result = new ArrayList<>();
boolean found = false;
for (Map.Entry<Long, StoredMessage> entry : cache.entrySet()) {
if (entry.getKey() >= fromOffset) {
if (entry.getKey() == fromOffset) {
found = true;
}
if (found) {
Comment on lines +709 to +718

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win

Cache lookup now requires an exact offset match, defeating the fast path for normal consumption.

Previously (per the AI summary) getMessagesFrom collected any cached entries with key >= fromOffset. Now it returns Collections.emptyList() immediately unless fromOffset is an exact cache key. Since globalOffset is a single counter shared across all topics (see append/appendBatch), a topic's next-offset-to-read essentially never matches an exact key in that topic's own cache except by coincidence. That means getMessages() will fall through to the disk-based scan (segment.read(...)) on nearly every call — turning the bounded in-memory cache into a no-op for the primary consume hot path.

⚡ Proposed fix restoring "collect from cached entries >= fromOffset"
         public List<StoredMessage> getMessagesFrom(long fromOffset, int maxCount) {
             lock.readLock().lock();
             try {
-                if (!cache.containsKey(fromOffset)) {
-                    return Collections.emptyList();
-                }
                 List<StoredMessage> result = new ArrayList<>();
-                boolean found = false;
                 for (Map.Entry<Long, StoredMessage> entry : cache.entrySet()) {
-                    if (entry.getKey() == fromOffset) {
-                        found = true;
-                    }
-                    if (found) {
+                    if (entry.getKey() >= fromOffset) {
                         result.add(entry.getValue());
                         if (result.size() >= maxCount) {
                             break;
                         }
                     }
                 }
                 return result;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (!cache.containsKey(fromOffset)) {
return Collections.emptyList();
}
List<StoredMessage> result = new ArrayList<>();
boolean found = false;
for (Map.Entry<Long, StoredMessage> entry : cache.entrySet()) {
if (entry.getKey() >= fromOffset) {
if (entry.getKey() == fromOffset) {
found = true;
}
if (found) {
List<StoredMessage> result = new ArrayList<>();
for (Map.Entry<Long, StoredMessage> entry : cache.entrySet()) {
if (entry.getKey() >= fromOffset) {
result.add(entry.getValue());
if (result.size() >= maxCount) {
break;
}
}
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@drmq-broker/src/main/java/com/drmq/broker/MessageStore.java` around lines 709
- 718, Update the cache lookup in getMessagesFrom to avoid requiring an exact
fromOffset key: iterate cached entries and collect those whose keys are greater
than or equal to fromOffset. Remove or adjust the containsKey(fromOffset) early
return so normal consumption can use the bounded cache fast path, while
preserving the existing result construction and ordering behavior.

result.add(entry.getValue());
if (result.size() >= maxCount) {
break;
Expand Down
8 changes: 8 additions & 0 deletions drmq-broker/src/main/java/com/drmq/broker/OffsetManager.java
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,14 @@ public int getOffsetEntryCount() {
return offsets.size();
}

/**
* Get a snapshot of all committed offsets across all groups and topics.
* @return map of "group/topic" to offset
*/
public Map<String, Long> getAllOffsets() {
return new java.util.HashMap<>(offsets);
}

/** Load all offsets from disk on startup. */
private void load() throws IOException {
if (!Files.exists(offsetsFile)) {
Expand Down
Loading