DanielCarter-stack commented on PR #10399:
URL: https://github.com/apache/seatunnel/pull/10399#issuecomment-3802910088
<!-- code-pr-reviewer -->
<!-- cpr:pr_reply_v2_parts {"group": "apache/seatunnel#10399", "part": 1,
"total": 1} -->
### Issue 1: WALCompactionDisruptor publishes complete events repeatedly
**Location**: `WALCompactionDisruptor.java:98-99`
**Severity**: MAJOR
**Category**: Performance/Design
**Issue**: Publishing complete events to two RingBuffers on every write
increases memory overhead; compaction threads do not need complete data
**Impact**: Increased memory usage in high-concurrency scenarios,
potentially affecting performance
### Issue 2: CloudLSMWriter frequently creates file streams
**Location**: `CloudLSMWriter.java:77`
**Severity**: MAJOR
**Category**: Performance
**Issue**: Creating a new FSDataOutputStream on every write() call, which is
unfriendly to cloud storage
**Impact**: Decreased write performance in cloud storage scenarios,
increased number of API calls
### Issue 3: Concurrent access to writeBatch and bf is unprotected
**Location**: `AbstractLSMWriter.java:68-69, 128-151`
**Severity**: CRITICAL
**Category**: Concurrency Safety
**Issue**: `writeBatch` and `bf` are accessed concurrently by multiple
threads without complete synchronization
**Impact**: May lead to `ConcurrentModificationException` or data corruption
### Issue 4: compaction() method lacks synchronization protection
**Location**: `AbstractLSMWriter.java:164-200`
**Severity**: MAJOR
**Category**: Concurrency Safety
**Issue**: Race condition exists between `compaction()` and `sortFlush`
**Impact**: May lead to inconsistent file state
### Issue 5: Delete markers not cleaned up during compaction
**Location**: `AbstractLSMWriter.java:232-235`
**Severity**: MAJOR
**Category**: Data Consistency
**Issue**: Delete markers (tombstone records) remain after compaction, not
thoroughly cleaned up
**Impact**: Accumulation of tombstone records after long-term operation,
wasting storage and affecting query performance
### Issue 6: Resource cleanup in exception handling is incomplete
**Location**: `CloudLSMWriter.java:95-105`
**Severity**: MINOR
**Category**: Error Handling
**Issue**: `bf` not cleaned up when exceptions occur
**Impact**: May lead to data duplication
### Issue 7: Insufficient concurrency test coverage
**Location**: `LSMWriterTest.java:327-396`
**Severity**: MINOR
**Category**: Test Coverage
**Issue**: Insufficient test intensity for concurrency scenarios
**Impact**: Concurrency bugs under edge conditions may go undetected in
testing
### Issue 8: CompactionFile comparison logic is incomplete
**Location**: `CompactionFile.java:40-54`
**Severity**: MINOR
**Category**: Code Quality
**Issue**: `compareTo`/`equals`/`hashCode` are inconsistent
**Impact**: May lead to abnormal `PriorityBlockingQueue` behavior
### Issue 9: Configuration error handling is not user-friendly
**Location**: `IMapFileStorage.java:174-186`
**Severity**: MINOR
**Category**: User Experience
**Issue**: JSON parsing error messages are not clear enough
**Impact**: Difficult to troubleshoot when configuration errors occur
--
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]
