Add comment to specify timeout unit in ConditionVariableTimedSleep()
Hi hackers,
ConditionVariableTimedSleep() accepts a timeout parameter, but it
doesn't explicitly state the unit for the timeout anywhere. To
determine this, one needs to look into the details of the function to
find it out from the comments of the internally called function
WaitLatch(). It would be beneficial to include a comment in the header
of ConditionVariableTimedSleep() specifying that the timeout is in
milliseconds, similar to what we have for other non-static functions
like WaitLatch and WaitEventSetWait. Attached the patch for the same.
thanks
Shveta
Attachments:
v1-0001-Specify-timeout-unit-in-ConditionVariableTimedSle.patchapplication/octet-stream; name=v1-0001-Specify-timeout-unit-in-ConditionVariableTimedSle.patchDownload
From 3ab99f9bb5687d2a5023225c66068325ae11e638 Mon Sep 17 00:00:00 2001
From: Shveta Malik <shveta.malik@gmail.com>
Date: Tue, 5 Mar 2024 09:28:09 +0530
Subject: [PATCH v1] Specify timeout unit in ConditionVariableTimedSleep
ConditionVariableTimedSleep() accepts timeout but does not
specify the unit of timeout anywhere. Introduce a comment
in the header indicating that the timeout parameter is
in milliseconds.
---
src/backend/storage/lmgr/condition_variable.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/src/backend/storage/lmgr/condition_variable.c b/src/backend/storage/lmgr/condition_variable.c
index 1e8559ed47..112a518bae 100644
--- a/src/backend/storage/lmgr/condition_variable.c
+++ b/src/backend/storage/lmgr/condition_variable.c
@@ -102,6 +102,8 @@ ConditionVariableSleep(ConditionVariable *cv, uint32 wait_event_info)
/*
* Wait for a condition variable to be signaled or a timeout to be reached.
*
+ * The "timeout" is given in milliseconds.
+ *
* Returns true when timeout expires, otherwise returns false.
*
* See ConditionVariableSleep() for general usage.
--
2.34.1
On Tue, Mar 05, 2024 at 09:39:11AM +0530, shveta malik wrote:
ConditionVariableTimedSleep() accepts a timeout parameter, but it
doesn't explicitly state the unit for the timeout anywhere. To
determine this, one needs to look into the details of the function to
find it out from the comments of the internally called function
WaitLatch(). It would be beneficial to include a comment in the header
of ConditionVariableTimedSleep() specifying that the timeout is in
milliseconds, similar to what we have for other non-static functions
like WaitLatch and WaitEventSetWait. Attached the patch for the same.
That sounds like a good idea to me, so I'm OK with your suggestion.
--
Michael
On Tue, Mar 05, 2024 at 03:20:48PM +0900, Michael Paquier wrote:
That sounds like a good idea to me, so I'm OK with your suggestion.
Applied this one as f160bf06f72a. Thanks.
--
Michael
On Sat, Mar 9, 2024 at 12:19 PM Michael Paquier <michael@paquier.xyz> wrote:
On Tue, Mar 05, 2024 at 03:20:48PM +0900, Michael Paquier wrote:
That sounds like a good idea to me, so I'm OK with your suggestion.
Applied this one as f160bf06f72a. Thanks.
Thanks!
thanks
Shveta