dataroaring commented on PR #55751:
URL: https://github.com/apache/doris/pull/55751#issuecomment-3284192130

   **📋 IMPROVEMENT: Missing Configuration Validation**
   
   **File**: `be/src/olap/tablet.cpp` lines 166-168
   
   ```cpp
   auto max_version_config = _tablet->max_version_config();
   int64_t version_count = _tablet->tablet_meta()->version_count();
   ```
   
   **Issue**: No validation that `max_version_config` is a reasonable/safe 
value.
   
   **Problems**:
   - Invalid configurations (≤ 0) could cause logic errors
   - Values ≤ 100 interact poorly with the hardcoded `-100` threshold
   - No fail-fast mechanism for misconfiguration
   
   **Potential Issues**:
   - `max_version_config = 0`: Version count check becomes meaningless
   - `max_version_config = 50`: The `version_count > max_version_config - 100` 
becomes `version_count > -50` (always true)
   - Negative values could cause integer underflow in calculations
   
   **Recommendation**: Add validation early in the method:
   ```cpp
   auto max_version_config = _tablet->max_version_config();
   
   // Validate configuration
   if (max_version_config <= 0) {
       LOG(ERROR) << "Invalid max_version_config: " << max_version_config 
                  << " for tablet: " << _tablet->tablet_id();
       return Status::InternalError("Invalid tablet version configuration: must 
be > 0");
   }
   
   if (max_version_config <= 100) {
       LOG(WARNING) << "max_version_config (" << max_version_config 
                    << ") is too low, may cause compaction issues";
       // Consider returning an error or using a minimum threshold
   }
   ```
   
   This provides:
   - Early detection of configuration errors
   - Clear error messages for troubleshooting
   - Protection against integer underflow scenarios


-- 
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]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to