apache / apache/rocketmq

[Enhancement] Tiered Storage: memory backpressure, concurrency hardening and bug fixes

Open
#10,462 2 comments 0 reactions 0 assignees View on GitHub
type/enhancement
Dominant language
Java
Stars
22.6k
Forks
12k
Avg merge
3d 1h
Merged PRs (30d)
27

Description

### Before Creating the Enhancement Request

- [x] I have confirmed that this should be classified as an enhancement rather than a bug/feature.

### Summary

Systematic improvements to the tieredstore module covering memory backpressure, concurrency safety, resource leak fixes, thread pool optimization, error handling, and log standardization.

### Motivation

The tieredstore module has several stability and data-safety risks:
- No backpressure mechanism — unbounded memory growth when dispatch falls behind, risking OOM
- PosixFileSegment FileChannel concurrent read/write race condition
- IndexStoreFile hash overflow on Integer.MIN_VALUE causing out-of-bounds access
- FileSegment close does not wait for in-flight commits, risking data loss
- MessageStoreExecutor thread pool too large (256+ threads on 32-core machines)
- Inconsistent log formatting makes troubleshooting difficult

### Describe the Solution You'd Like

**Memory backpressure**: add ratio + cap backpressure (default 10% heap, capped at 1GB), check available memory before dispatch.

**Concurrency fixes**:
- PosixFileSegment: use `read(ByteBuffer, long)` for atomic reads
- IndexStoreFile: hash with `& 0x7FFFFFFF` to prevent MIN_VALUE overflow
- FlatCommitLogFile: local variable snapshot for firstOffset to prevent TOCTOU
- FlatMessageFile: add `volatile` to metadata fields

**Resource management**:
- FileSegment `close()` waits for in-flight commit (30s timeout)
- PosixFileSegment: store RAF references to prevent handle leaks
- MessageStoreExecutor `shutdown()`: add `awaitTermination(30s)`
- Remove MessageStoreExecutor singleton pattern and `fileRecyclingExecutor`
- FlatFileFactory: remove test constructor that leaks thread pool

**Thread pool optimization**: core = `processors`, max = `processors * 2`, remove `fileRecyclingExecutor`.

**Error handling**:
- TieredMessageStore: unwrap `CompletionException`, re-throw `Error` instead of swallowing
- PosixFileSegment: throw `RuntimeException` on IOException instead of returning empty buffer
- FlatAppendFile: `getFileCorrectSize` changed to bounded 3-retry
- IndexStoreFile: `timeDiff` clamped to `[0, Integer.MAX_VALUE]`

**Log standardization**: unify to `ClassName#methodName, description, key={}` format.

### Describe Alternatives You've Considered

N/A

### Additional Context

N/A

Contributor guide

Open the contributing guide

Research direction

Start by reading the tieredstore module and the named classes, including PosixFileSegment, IndexStoreFile, FileSegment, MessageStoreExecutor, FlatFileFactory, and TieredMessageStore. Break the work into the listed backpressure, concurrency, resource, executor, error-handling, and logging changes, and consider each item done only when its stated behavior is implemented and the module remains safe under concurrent and failing operations.

Written by the indexing model from the issue text.

Assessment

Tech stack
java
Domain
backend, distributed-systems, performance
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.