RockteMQ-AI commented on code in PR #2619:
URL:
https://github.com/apache/rocketmq-dashboard/pull/2619#discussion_r3859451615
##########
server/src/main/java/org/apache/rocketmq/studio/instance/dlq/DLQController.java:
##########
@@ -95,22 +97,25 @@ public Result<PageResult<DLQMessageVO>>
listDLQMessages(@PathVariable String gro
}
@GetMapping("/export-excel")
- public ResponseEntity<byte[]> exportDLQExcel(@RequestParam String
instanceId,
- @RequestParam String
groupName,
- @RequestParam(required =
false) Long startTime,
- @RequestParam(required =
false) Long endTime,
- @Size(max =
MAX_SELECTED_MESSAGES,
- message = "At most
100 msgIds are allowed per export")
- @RequestParam(required =
false) List<String> msgIds) {
- DLQExcelExportResultVO result = dlqService.exportExcel(instanceId,
groupName, startTime, endTime, msgIds);
- return ResponseEntity.ok()
- .header(HttpHeaders.CONTENT_DISPOSITION,
- attachmentDisposition("dlq-" +
sanitizeForFilename(groupName) + ".xlsx").toString())
- .header(HEADER_EXPORT_TRUNCATED,
String.valueOf(result.isTruncated()))
- .header(HEADER_EXPORT_FAILED_QUEUES,
String.valueOf(result.getFailedQueueCount()))
- .header(HEADER_EXPORT_LIMIT, String.valueOf(result.getLimit()))
- .contentType(MediaType.parseMediaType(EXCEL_MEDIA_TYPE))
- .body(result.getData());
+ public ResponseEntity<Void> exportDLQExcel(@RequestParam String instanceId,
+ @RequestParam String groupName,
+ @RequestParam(required = false)
Long startTime,
+ @RequestParam(required = false)
Long endTime,
+ @Size(max =
MAX_SELECTED_MESSAGES,
+ message = "At most 100
msgIds are allowed per export")
+ @RequestParam(required = false)
List<String> msgIds,
+ HttpServletResponse response)
throws IOException {
+ // Stream the workbook straight to the response body so it is never
buffered in the heap.
+ response.setContentType(EXCEL_MEDIA_TYPE);
+ response.setHeader(HttpHeaders.CONTENT_DISPOSITION,
+ attachmentDisposition("dlq-" + sanitizeForFilename(groupName)
+ ".xlsx").toString());
Review Comment:
**[Warning]** Response headers set after streaming write.\n\nThe metadata
headers (`X-DLQ-Export-Truncated`, `X-DLQ-Export-FailedQueues`,
`X-DLQ-Export-Limit`) are set **after** `dlqService.exportExcel()` writes to
`response.getOutputStream()`. In a real servlet container, if the Excel data
exceeds the response buffer (typically 8 KB for Tomcat), the buffer
auto-flushes during the write and the response becomes committed — subsequent
`setHeader()` calls are silently ignored.\n\nFor large exports, the client
would lose the completeness metadata headers, which defeats their purpose (the
client uses them to detect truncation and failed queues).\n\n**Suggested fix:**
Set all headers **before** the streaming write. Since the metadata is only
available after the export completes, consider a two-phase
approach:\n```java\n// Phase 1: write to a temp buffer / piped stream to
collect metadata\n// Phase 2: set headers from metadata\n// Phase 3: pipe
buffer to response output\n```\nOr restr
ucture `exportExcel` to return metadata before writing the body.
##########
server/src/main/java/org/apache/rocketmq/studio/provider/apache/RocketMQDLQProvider.java:
##########
@@ -335,23 +334,24 @@ public DLQExcelExportResultVO exportExcel(String
instanceId, String groupName, L
DeadLetterScanResult scanResult = collectDeadLetters(instanceId,
dlqTopic, begin, end, RESEND_HARD_CAP);
Set<String> selected = msgIds == null ? Collections.emptySet()
: new java.util.HashSet<>(msgIds);
- List<DLQMessageVO> messages = scanResult.messages().stream()
+ List<DLQMessageExcelRow> rows = scanResult.messages().stream()
.filter(message -> selected.isEmpty() ||
selected.contains(message.getMsgId()))
.map(this::toExportVO)
+ .map(DLQMessageExcelRow::from)
.toList();
- byte[] data;
try {
- ByteArrayOutputStream output = new ByteArrayOutputStream();
- com.alibaba.excel.EasyExcel.write(output, DLQMessageExcelRow.class)
+ // Stream straight to the caller's OutputStream so the whole
workbook is never buffered
+ // in the heap (the export is capped at RESEND_HARD_CAP messages,
but large snapshots can
+ // still be several MB).
+ com.alibaba.excel.EasyExcel.write(out, DLQMessageExcelRow.class)
.sheet("DLQ")
-
.doWrite(messages.stream().map(DLQMessageExcelRow::from).toList());
- data = output.toByteArray();
+ .doWrite(rows);
+ out.flush();
} catch (Exception e) {
log.warn("Failed to build Excel export for group {}: {}",
groupName, e.getMessage());
Review Comment:
**[Info]** Redundant flush. `out.flush()` is called here (line 351) and
again in the controller (`response.getOutputStream().flush()`). The double
flush is harmless but unnecessary — one flush at the controller level is
sufficient.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
