Fix misleading QuotaConfig error messages and reject non-finite maxQueriesPerSecond

[dkranchii]

Summary

QuotaConfig produces misleading validation errors when a table config has a bad storage or maxQueriesPerSecond value. Four defects fixed:

1. Wrong variable interpolated. For a bad maxQueriesPerSecond, the code threw "Invalid 'maxQueriesPerSecond': " + storage, so the operator saw the storage value (or null) instead of the offending QPS input.
2. Missing exception cause. Both IllegalArgumentException throws dropped the underlying NumberFormatException / parse exception, so stack traces had no root cause.
3. Message-less Preconditions.checkArgument. Preconditions.checkArgument(_maxQPS > 0) had no message; a non-positive QPS produced an IllegalArgumentException with a null message.
4. "Infinity" bypass. Double.parseDouble("Infinity") returns POSITIVE_INFINITY, which passes > 0 — an undocumented back-door for unlimited QPS. "NaN" was also silently reaching downstream code.

Changes

• pinot-spi/src/main/java/org/apache/pinot/spi/config/table/QuotaConfig.java (+4 / -3):
  • Interpolate maxQueriesPerSecond (not storage) in the QPS error message.
  • Pass the causing exception e on both IllegalArgumentException throws.
  • Give the Preconditions.checkArgument a descriptive message that includes the offending value.
  • Tighten the positivity check to Double.isFinite(_maxQPS) && _maxQPS > 0 so Infinity, -Infinity, and NaN are all rejected.
• pinot-spi/src/test/java/org/apache/pinot/spi/config/table/QuotaConfigTest.java (+109, tests only) — six new focused tests covering the field name, offending value, cause chain, zero boundary, non-finite inputs, and the Jackson deserialization path.

Backward compatibility

• No public API signature or serialization format is changed.
• Error message tis soft public surface. Callers that string-matched on "Invalid 'maxQueriesPerSecond': null" (the buggy pre-fix message) will need to update, but the corrected format is what the code clearly intended.
• "Infinity" / "NaN" inputs now fail validation. If any deployment relied on "Infinity" as a shortcut for "unlimited", they should omit the field instead (which already means "unlimited" via INVALID_MAX_QPS = -1.0).

Note on overlap with #16016

PR #16016 (a much larger, ~1-year-old feature refactor: "Enable Ratelimiter Quota with Flexible Configuration") also modifies QuotaConfig.java and QuotaConfigTest.java. That PR is a feature refactor with orthogonal concerns; this PR is a focused bug fix on the existing validation error paths. Happy to rebase whichever merges second.

Test plan

• New regression tests in QuotaConfigTest covering all four defects, plus boundary cases (0, Infinity, -Infinity, NaN) and the Jackson deserialization path.
• CI will run ./mvnw -pl pinot-spi -am -Dtest=QuotaConfigTest test plus spotless / checkstyle / license.

[Copilot]

Pull request overview

This pull request fixes QuotaConfig validation so operators get accurate, actionable errors when storage or maxQueriesPerSecond are invalid, and it closes a loophole where non-finite QPS values (Infinity/NaN) could bypass validation.

Changes:

• Fixes the maxQueriesPerSecond validation error to interpolate the correct input value and to preserve the underlying exception as the cause.
• Makes the QPS validation explicitly reject non-finite numbers (Double.isFinite(...) && > 0) and provides a non-null checkArgument message.
• Adds focused unit tests covering message contents, cause chaining, zero/non-positive boundaries, non-finite inputs, and the Jackson deserialization path.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
pinot-spi/src/main/java/org/apache/pinot/spi/config/table/QuotaConfig.java	Corrects error messages/cause propagation and rejects non-finite/invalid QPS inputs during config validation.
pinot-spi/src/test/java/org/apache/pinot/spi/config/table/QuotaConfigTest.java	Adds regression tests to ensure validation errors surface the right field/value and preserve exception causes (including JSON deserialization).

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 66.95%. Comparing base (2032d7a) to head (e530e7f).

Additional details and impacted files

@@            Coverage Diff            @@
##             master   #19237   +/-   ##
=========================================
  Coverage     66.94%   66.95%           
  Complexity     1423     1423           
=========================================
  Files          3452     3452           
  Lines        218564   218565    +1     
  Branches      34731    34731           
=========================================
+ Hits         146320   146339   +19     
+ Misses        60549    60536   -13     
+ Partials      11695    11690    -5     

Flag	Coverage Δ
integration	100.00% <ø> (ø)
integration1	100.00% <ø> (ø)
integration2	0.00% <ø> (ø)
java-25	66.95% <100.00%> (+<0.01%)	⬆️
lane-a	100.00% <ø> (ø)
lane-b	0.00% <ø> (ø)
temurin	66.95% <100.00%> (+<0.01%)	⬆️
unittests	66.95% <100.00%> (+<0.01%)	⬆️
unittests1	57.70% <100.00%> (+<0.01%)	⬆️
unittests2	39.03% <25.00%> (+<0.01%)	⬆️

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

☔ View full report in Codecov by Harness.
📢 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.