[ingestion] Add rate-limited exception logging for transformers #17239

suvodeep-pyne · 2025-11-19T23:01:42Z

Introduces ThrottledLogger utility to provide rate-limited exception logging for record transformers, solving the problem where high-frequency errors pollute logs and starve low-frequency critical errors.

Key Changes

Use throttled logging by default to log 5 exceptions per exception class per min
Remove record logging in transformers in case of exceptions. This prevents both performance and data sharing related issues

Core Implementation

New ThrottledLogger utility
- Implements per-exception-class rate limiting using custom Token Bucket algorithm
- Each exception type gets independent rate limiter (So we still at least see which exception with min perf overhead)
- Non-thread-safe by design (needs to be fast)
- Falls back to DEBUG logging when rate=0 (backward compatible behavior)
New RateLimiter class
- Token Bucket implementation
- No additional dependency
- Designed for fast single-threaded access
Updated 6 transformers to use throttled logger:
- FilterTransformer
- DataTypeTransformer
- ExpressionTransformer
- ComplexTypeTransformer
- SchemaConformingTransformer
- TimeValidationTransformer
New configuration: IngestionConfig.ingestionExceptionLogRateLimitPerMin (default: 5)

Testing

Added tests for RateLimiter and ThrottledLogger to cover important cases.

Benefits

Solves Noisy Neighbor Problem: High-frequency NumberFormatException won't starve low-frequency ConnectException
Production Visibility: When configured, exceptions log at WARN/ERROR level instead of DEBUG
Controlled Log Volume: Prevents log flooding during data quality issues
Zero Dependencies: Uses custom Token Bucket implementation, no Guava required
Efficient: Package-private implementation, minimal allocation overhead

Example Output

WARN  [FilterTransformer] Caught exception while executing filter function...
java.lang.NumberFormatException: For input string: "abc"

[... 4 more similar logs within 1 minute ...]

[After rate limit window passes]
WARN  [FilterTransformer] Dropped 9995 occurrences of NumberFormatException
WARN  [FilterTransformer] Caught exception while executing filter function...

Meanwhile, different exception types (e.g., ConnectException) log immediately using independent rate limiters.

Implementation Details

API simplified: Transformers pass IngestionConfig directly to logger
Rate conversion logic (per-min → tokens/nanosecond) encapsulated in RateLimiter
Memory-bounded: Exception class count typically 10-50
Token bucket algorithm provides smooth rate limiting with burst handling
@VisibleForTesting constructor allows controlled time in tests

- Simplified API: PinotThrottledLogger now accepts IngestionConfig and table name directly, eliminating duplicated rate conversion logic across transformers - Backward compatible: Changed default ingestionExceptionLogRateLimitPerMin from 5 to 0; rate=0 falls back to DEBUG logging (original behavior) - Added ServerMeter emission: LOGS_DROPPED_BY_THROTTLED_LOGGER metric tracks suppressed logs per table when table name is provided - Updated all transformers: FilterTransformer, DataTypeTransformer, ComplexTypeTransformer, ExpressionTransformer, SchemaConformingTransformer, TimeValidationTransformer now use simplified constructor - Test updates: PinotThrottledLoggerTest updated to verify DEBUG fallback behavior when rate=0

…dLogger Key changes: - Log throttled exceptions at DEBUG level to ensure no exception info is lost - Simplify from ConcurrentHashMap/AtomicLong to HashMap/long for single-threaded usage - Combine two maps into one ExceptionState wrapper for single lookup - Update javadoc to clarify three-level logging behavior - Remove thread-safety test (single-threaded per TransformPipeline instance) Logging behavior after this change: - Rate ≤ 0: All logs at DEBUG (backward compatible) - Rate > 0, within quota: WARN/ERROR with suppression counts - Rate > 0, quota exceeded: DEBUG while tracking suppression counts

codecov-commenter · 2025-11-20T01:21:28Z

Codecov Report

❌ Patch coverage is 88.31169% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 63.19%. Comparing base (3224ce6) to head (13181e8).
⚠️ Report is 49 commits behind head on master.

Files with missing lines	Patch %	Lines
...org/apache/pinot/common/utils/ThrottledLogger.java	86.20%	3 Missing and 1 partial ⚠️
...ocal/recordtransformer/ComplexTypeTransformer.java	50.00%	2 Missing ⚠️
...recordtransformer/SchemaConformingTransformer.java	50.00%	1 Missing ⚠️
...l/recordtransformer/TimeValidationTransformer.java	66.66%	1 Missing ⚠️
...va/org/apache/pinot/spi/utils/CommonConstants.java	0.00%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##             master   #17239      +/-   ##
============================================
+ Coverage     63.17%   63.19%   +0.01%     
- Complexity     1428     1432       +4     
============================================
  Files          3121     3133      +12     
  Lines        184814   185904    +1090     
  Branches      28332    28401      +69     
============================================
+ Hits         116760   117473     +713     
- Misses        59033    59366     +333     
- Partials       9021     9065      +44

Flag	Coverage Δ
custom-integration1	`100.00% <ø> (ø)`
integration	`100.00% <ø> (ø)`
integration1	`100.00% <ø> (ø)`
integration2	`0.00% <ø> (ø)`
java-11	`63.16% <88.31%> (+0.01%)`	⬆️
java-21	`63.15% <88.31%> (+<0.01%)`	⬆️
temurin	`63.19% <88.31%> (+0.01%)`	⬆️
unittests	`63.18% <88.31%> (+0.01%)`	⬆️
unittests1	`55.59% <77.92%> (-0.36%)`	⬇️
unittests2	`33.87% <68.83%> (+0.17%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Jackie-Jiang

Mostly good. Please update the PR description about the concurrency part

pinot-common/src/main/java/org/apache/pinot/common/utils/PinotThrottledLogger.java

Jackie-Jiang · 2025-11-24T22:11:26Z

pinot-common/src/main/java/org/apache/pinot/common/utils/PinotThrottledLogger.java

+  }
+
+  private void logWithRateLimit(String msg, Throwable t, BiConsumer<String, Throwable> consumer) {
+    if (_permitsPerSecond <= 0) {


Consider modeling this as a separate DebugOnlyLogger. We can also have a UnthrottledLogger if necessary

pinot-common/src/main/java/org/apache/pinot/common/utils/PinotThrottledLogger.java

pinot-common/src/main/java/org/apache/pinot/common/metrics/ServerMeter.java

…ensive unit tests. Simplify `ThrottledLogger` by removing inlined RateLimiter and update related tests.

Jackie-Jiang

LGTM otherwise

Jackie-Jiang · 2025-11-24T23:51:30Z

...l/src/main/java/org/apache/pinot/segment/local/recordtransformer/ComplexTypeTransformer.java

        }
-        LOGGER.debug("Caught exception while transforming complex types for record: {}", record.toString(), e);
+        _throttledLogger.warn(
+            String.format("Caught exception while transforming complex types for record: %s", record.toString()), e);


Avoid using String.format as it is quite expensive. Concatanation should be cheaper.

Given LOGGER.debug() doesn't do anything when debug level log is not enabled, and we are already paying overhead of constructing the string, we should consider either:

Expose acquire() from _throttledLogger

Pass arguments the same way as what LOGGER expects

Not introduced in this PR, but we shouldn't call record.toString(). Instead, we should directly pass record

Removed record logging itself. that address both performance and data risks.

pinot-spi/src/main/java/org/apache/pinot/spi/utils/CommonConstants.java

… and consistency

…ving record details for clarity and consistency.

suvodeep-pyne added 5 commits November 19, 2025 14:58

Update PinotThrottledLogger example for API consistency and clarity

0e22ac5

Simplify suppression message format in PinotThrottledLogger for clarity

d64a79e

Fix checkstyle

2b78875

Jackie-Jiang added feature ingestion observability labels Nov 24, 2025

Jackie-Jiang reviewed Nov 24, 2025

View reviewed changes

suvodeep-pyne added 2 commits November 24, 2025 14:53

Addressing comments

d621a62

Extract RateLimiter implementation to its own class and add compreh…

4aa3f0f

…ensive unit tests. Simplify `ThrottledLogger` by removing inlined RateLimiter and update related tests.

Jackie-Jiang approved these changes Nov 24, 2025

View reviewed changes

suvodeep-pyne commented Nov 24, 2025

View reviewed changes

pinot-spi/src/main/java/org/apache/pinot/spi/utils/CommonConstants.java Show resolved Hide resolved

suvodeep-pyne changed the title ~~Add rate-limited exception logging for transformers~~ [ingestion] Add rate-limited exception logging for transformers Nov 25, 2025

suvodeep-pyne added 2 commits November 24, 2025 21:00

Remove record details from exception logs in transformers for clarity…

b1a88ea

… and consistency

Simplify exception log message in SchemaConformingTransformer by remo…

13181e8

…ving record details for clarity and consistency.

Jackie-Jiang merged commit d78b39b into apache:master Nov 25, 2025
18 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[ingestion] Add rate-limited exception logging for transformers #17239

[ingestion] Add rate-limited exception logging for transformers #17239

Uh oh!

suvodeep-pyne commented Nov 19, 2025 •

edited

Loading

Uh oh!

codecov-commenter commented Nov 20, 2025 •

edited

Loading

Uh oh!

Jackie-Jiang left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Jackie-Jiang Nov 24, 2025

Uh oh!

Uh oh!

Uh oh!

Jackie-Jiang left a comment

Uh oh!

Jackie-Jiang Nov 24, 2025

Uh oh!

suvodeep-pyne Nov 25, 2025

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

[ingestion] Add rate-limited exception logging for transformers #17239

[ingestion] Add rate-limited exception logging for transformers #17239

Uh oh!

Conversation

suvodeep-pyne commented Nov 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Key Changes

Core Implementation

Testing

Benefits

Example Output

Implementation Details

Uh oh!

codecov-commenter commented Nov 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Jackie-Jiang left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Jackie-Jiang Nov 24, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Jackie-Jiang left a comment

Choose a reason for hiding this comment

Uh oh!

Jackie-Jiang Nov 24, 2025

Choose a reason for hiding this comment

Uh oh!

suvodeep-pyne Nov 25, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

suvodeep-pyne commented Nov 19, 2025 •

edited

Loading

codecov-commenter commented Nov 20, 2025 •

edited

Loading