haubur commented on PR #3891: URL: https://github.com/apache/iggy/pull/3891#issuecomment-5307012832
The problem is real for consumers: - panic on a 0 IggyDuration: https://github.com/apache/iggy/blob/ec456d2370de25f6585dc245077a487d9cdf7730/core/sdk/src/clients/consumer.rs#L328 - and (not) sleeping with 0 on the background store offset at interval task loop. : https://github.com/apache/iggy/blob/ec456d2370de25f6585dc245077a487d9cdf7730/core/sdk/src/clients/consumer.rs#L496 and producers: - panic on a 0 IggyDuration: https://github.com/apache/iggy/blob/ec456d2370de25f6585dc245077a487d9cdf7730/core/sdk/src/clients/producer.rs#L265 Clarification: - The problem occurs if **manually** setting a IggyDuration equal to zero. - **No default path** actually sets an IggyDuration with zero. - but with mentioned CLI call `--tcp-heartbeat-interval none` the panics are triggered Solution: - Having IggyDuration falling to 0 for "unlimited", "disabled" and "none" is a little misleading. - Since it's a common type there might be unobserved behavior in other places should we change it now. - However, **I would prefer not checking in methods calls as done in the PR but enforcing the proposed guards on a type level e.g. by introducing a NonZeroIggyDuration.** -- 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]
