Fix bug where the timeout in the admin APIs wasn't taken into account.

[benjaminroux]

The timeout_ms parameter for the admin APIs wasn't being used anywhere.
With that change, the parameter is now used to determine how many times we are going to poll for the given event.
I didn't change the polling timeout (1000ms) so I had to make a decision to round down to the second (i.e. 1800ms would be rounded down to 1sec) and also overriding any value < 1000 to 1000.

Since that parameter wasn't being used yet, another solution would be to change the granularity from ms to sec in the function (timeout_sec instead of timeout_ms). This would prevent the rounding down and the minimum value tricks.

[webmakersteve]

Is there a reason we need to pass a full second interval into the librdkafka poll functions? If we do need to keep it rounding like that i would prefer that we just explicitly only take seconds rather than milliseconds, but I don't see the reason why we can't allow timeouts to be provided in milliseconds. The polling timeout can = the timeout provided, or an interval of it so x number of polls yields the full timeout if we need it to happen more granularly than a second but also want to do work between invocations of the poll.

[benjaminroux]

Is there a reason we need to pass a full second interval into the librdkafka poll functions? If we do need to keep it rounding like that i would prefer that we just explicitly only take seconds rather than milliseconds

I agree however I didn't want to change the prototype of the functions since people might already be passing milliseconds timeout (even though it's not really working) which would suddenly become seconds.

but I don't see the reason why we can't allow timeouts to be provided in milliseconds. The polling timeout can = the timeout provided, or an interval of it so x number of polls yields the full timeout if we need it to happen more granularly than a second but also want to do work between invocations of the poll.

I guess that's where people who know the codebase might help.
I'm not exactly sure how the polling event thing works. I saw that a retry mechanism was already used (polling 5 times for 1000ms, instead of just polling for 5000ms) so I wasn't sure if having "n loop of 1000ms" was the same as having "1 loop of n*1000ms".

If you could advise me on that I could make the appropriate change.

Thanks.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

[anonimitoraf]

I think I just ran into this issue. What's being awaited for this to be merged?

@iradul

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

[iradul]

In some edge cases, fixed 1000ms polling timeout may not be long enough. This is why I made it exponential so that it doubles after each attempt. The initial number of attempts is calculated based on the total timeout provided.
For example, if the provided timeout is 2000ms or less, there will be only a single poll with that same timeout.
If the timeout is 15000ms, there will be 4 poll attempts, starting with 1000ms followed by 2000ms, 4000ms and finally 8000ms timeout.

[iradul]

Thanks, @benjaminroux!