Repository navigation
Conversation
The size and time types matched only the digits right before the unit, so the integer part of a fractional value was dropped silently: "0.5s" became 5 seconds, "1.5m" 5 minutes and "1.5k" 5120 bytes. Accept a decimal number before the unit. Values without a fractional part are parsed exactly as before and keep returning an Integer. Signed-off-by: Lucas Barros <lucasbarros.kam@gmail.com>
Watson1978
left a comment
There was a problem hiding this comment.
Thanks for the PR and the clear description. Reading 0.5s as 5 seconds is an easy trap to fall into, and the fix makes sense.
One thing to fix before merging: unit_value uses Float arithmetic, which is not exact for many decimal inputs and can shift timekey buckets. Details and a suggestion are inline.
About compatibility: existing configurations that already use such values get a different effective value after upgrading. For example, total_limit_size 2.5g was 5 GiB and becomes 2.5 GiB. So we will ship this in the next minor release as an enhancement, not as a backport to v1.19, and call it out in the release note.
| # "10" stays an Integer, "1.5" becomes a Float, so values without a | ||
| # fractional part keep the same type as before. | ||
| def self.unit_value(number, multiplier) | ||
| if number.include?('.') | ||
| number.to_f * multiplier | ||
| else | ||
| number.to_i * multiplier | ||
| end | ||
| end |
There was a problem hiding this comment.
SHOULD: number.to_f * multiplier is not exact for many decimal inputs, even when the result is a whole number of seconds:
"1.1h" => 3960.0000000000005
"0.7d" => 60479.99999999999
"4.1m" => 245.99999999999997
This matters for timekey. For example, with timekey_use_utc true and timekey 0.7d, an event at 2026-07-16 00:00:10 UTC gets the chunk timekey 2026-07-15 23:59:59 instead of 2026-07-16 00:00:00. So time placeholders in path can point to the wrong minute, hour or day. With timekey 1.1h, an event at the exact boundary second goes to the previous chunk.
Multiplying with Rational avoids this. Whole results become exact Integers, like 10s today, and the others become Floats:
| # "10" stays an Integer, "1.5" becomes a Float, so values without a | |
| # fractional part keep the same type as before. | |
| def self.unit_value(number, multiplier) | |
| if number.include?('.') | |
| number.to_f * multiplier | |
| else | |
| number.to_i * multiplier | |
| end | |
| end | |
| # Rational avoids Float errors: "1.1h" is 3960, not 3960.0000000000005. | |
| def self.unit_value(number, multiplier) | |
| value = number.to_r * multiplier | |
| value.denominator == 1 ? value.to_i : value.to_f | |
| end |
With this, 1.5m returns 90 instead of 90.0. The current test data still passes, because assert_equal compares with ==.
Could you also add these cases to the Config.time_value normal case data? They fail with the current code:
"1.1h" => [3960, "1.1h"],
"0.7d" => [60480, "0.7d"],
Which issue(s) this PR fixes:
None (found while reading
lib/fluent/config/types.rb; no existing issue or PR about it).What this PR does / why we need it:
Config.size_valueandConfig.time_valuelook for the unit with unanchored regexps such as/([0-9]+)s/. With a fractional value, only the digits right before the unit are captured, so the integer part is dropped without any warning:flush_interval 0.5sgives a 5 second flush interval,retry_wait 1.5mwaits 5 minutes,chunk_limit_size 1.5mgives 5 MB. A plain float (flush_interval 0.5) already works because it falls through toFloat(), so this mostly bites people who add the unit.This accepts an optional decimal part before the unit (
/([0-9]+(?:\.[0-9]+)?)s/, and the same for the other units). Values without a fractional part go through the same arithmetic as before and still return an Integer, so10s,10sec,2m,4Gand friends are unchanged. Sizes are truncated to an Integer (1.5k-> 1536), as thesizetype is a byte count.Tests: new data in the
normal casetests ofConfig.size_value(1.5k,0.5m,2.5G) andConfig.time_value(0.5s,1.5m,0.25h,1.5d), checked with and withoutstrict. They fail on master and pass here:test/config/test_configurable.rb,test_configure_proxy.rb,test_system_config.rb,test/plugin/test_buffer.rbandtest_output_as_buffered_retries.rbpass too.test/config/test_config_parser.rb,test_dsl.rbandtest/test_config.rbhave the same failures with and without this change on my machine (include paths relative to the working directory).Docs Changes:
https://docs.fluentd.org/configuration/config-file#supported-data-types-for-values lists
<INTEGER>k,<INTEGER>sand so on. If this is accepted, it could say that a decimal number is accepted before the unit (e.g.0.5s,1.5k).Release Note:
config: a fractional number before a size or time unit (
0.5s,1.5m,1.5k) is parsed as such instead of dropping the integer part