Jackie-Jiang opened a new pull request, #16642:
URL: https://github.com/apache/pinot/pull/16642
### Summary
This PR reduces excessive logging in the
`PerQueryCPUMemResourceUsageAccountant` class to improve performance and reduce
log noise in production environments. The changes focus on converting verbose
WARN-level logs to DEBUG-level logs while maintaining essential error
information.
### Changes Made
#### ๐ง **Core Improvements**
- **Reduced log verbosity**: Changed several WARN-level logs to DEBUG-level
to reduce log noise
- **Improved thread safety**: Replaced `HashSet` with
`ConcurrentHashMap.newKeySet()` for `_cancelSentQueries`
- **Enhanced error handling**: Improved interrupt handling in the
`WatcherTask` run loop
- **Code cleanup**: Removed unused parameters and improved code structure
#### ๐ **Specific Changes**
1. **Logging Level Changes**:
- `logQueryResourceUsage()`: WARN โ DEBUG for query aggregation results
- `logSelfTerminatedQuery()`: Improved logic and formatting
- `triggeredActions()`: WARN โ DEBUG for alarming level verbose logging
- Various query termination logs: Improved formatting and clarity
2. **Thread Safety Improvements**:
- Replaced `HashSet` with `ConcurrentHashMap.newKeySet()` for better
concurrency
- Improved interrupt handling in `WatcherTask.run()`
3. **Code Structure**:
- Removed unused `isThreadSamplingEnabledForMSE` parameter
- Added `@VisibleForTesting` annotation for test methods
- Made `TriggeringLevel` enum protected for better encapsulation
- Improved trigger evaluation logic with better state management
4. **Test Updates**:
- Updated test classes to match new constructor signatures
- Removed excessive log4j configuration from integration tests
- Updated assertion messages to match new log formats
#### ๐งช **Files Modified**
- `PerQueryCPUMemAccountantFactory.java` - Main changes to reduce logging
and improve thread safety
- `QueryAggregator.java` - Updated documentation
- `TestResourceAccountant.java` - Updated constructor call
- Integration test files - Removed excessive logging setup and updated
assertions
### Impact
- **Performance**: Reduced log I/O overhead in production environments
- **Maintainability**: Cleaner code structure with better separation of
concerns
- **Debugging**: Essential error information is still logged at appropriate
levels
- **Thread Safety**: Improved concurrency handling for query cancellation
tracking
### Testing
- All existing integration tests pass with updated assertions
- Thread safety improvements tested through existing concurrent test
scenarios
- Log level changes verified to maintain essential debugging information
--
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]
