Skip to content

Use CLOCK_MONOTONIC_RAW instead of CLOCK_MONOTONIC #5099

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Sep 29, 2025
Merged

Use CLOCK_MONOTONIC_RAW instead of CLOCK_MONOTONIC #5099

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
daf1570
Replace `CLOCK_MONOTONIC` with `CLOCK_MONOTONIC_RAW`
wazir-ahmed Sep 2, 2025
8d29742
`QueryInfo::end_time` should always be greater than or equal to `star…
wazir-ahmed Sep 3, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 25 additions & 0 deletions include/MySQL_Session.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,8 +96,33 @@ class Query_Info {
void end();
char *get_digest_text();
bool is_select_NOT_for_update();
void set_end_time(unsigned long long time);
};

/**
* @brief Assigns query end time.
* @details In addition to being a setter for end_time member variable, this
* method ensures that end_time is always greater than or equal to start_time.
* Refer https://github.com/sysown/proxysql/issues/4950 for more details.
* @param time query end time
*/
inline void Query_Info::set_end_time(unsigned long long time) {
end_time = time;

#ifndef CLOCK_MONOTONIC_RAW
if (start_time <= end_time)
return;

// If start_time is greater than end_time, assign current monotonic time
end_time = monotonic_time();
if (start_time <= end_time)
return;

// If start_time is still greater than end_time, set the difference to 0
end_time = start_time;
#endif // CLOCK_MONOTONIC_RAW
}
Comment on lines +109 to +124
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The implementation of set_end_time is identical to the one in PgSQL_Query_Info::set_end_time. To improve maintainability and avoid code duplication, this logic should be extracted into a shared helper function. For example, you could create a helper function in gen_utils.h and call it from both methods.


/**
* @class MySQL_Session
* @brief Manages a client session, including query parsing, backend connections, and state transitions.
Expand Down
25 changes: 25 additions & 0 deletions include/PgSQL_Session.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,12 +179,37 @@ class PgSQL_Query_Info {
void end();
char* get_digest_text();
bool is_select_NOT_for_update();
void set_end_time(unsigned long long time);

private:
void reset_extended_query_info();
void init(unsigned char* _p, int len, bool header = false);
};

/**
* @brief Assigns query end time.
* @details In addition to being a setter for end_time member variable, this
* method ensures that end_time is always greater than or equal to start_time.
* Refer https://github.com/sysown/proxysql/issues/4950 for more details.
* @param time query end time
*/
inline void PgSQL_Query_Info::set_end_time(unsigned long long time) {
end_time = time;

#ifndef CLOCK_MONOTONIC_RAW
if (start_time <= end_time)
return;

// If start_time is greater than end_time, assign current monotonic time
end_time = monotonic_time();
if (start_time <= end_time)
return;

// If start_time is still greater than end_time, set the difference to 0
end_time = start_time;
#endif // CLOCK_MONOTONIC_RAW
}
Comment on lines +196 to +211
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The implementation of set_end_time is duplicated in Query_Info::set_end_time. To improve maintainability and avoid code duplication, this logic should be extracted into a shared helper function. For example, you could create a helper function in gen_utils.h and call it from both methods.


class PgSQL_Session : public Base_Session<PgSQL_Session, PgSQL_Data_Stream, PgSQL_Backend, PgSQL_Thread> {
private:
using PktType = std::variant<std::unique_ptr<PgSQL_Parse_Message>,std::unique_ptr<PgSQL_Describe_Message>,
Expand Down
7 changes: 6 additions & 1 deletion include/gen_utils.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,11 @@ class FixedSizeQueue : public std::queue<buffer_t> {
#endif /* __CLASS_PTR_ARRAY_H */


#ifdef CLOCK_MONOTONIC_RAW
#define PROXYSQL_CLOCK_MONOTONIC CLOCK_MONOTONIC_RAW
#else
#define PROXYSQL_CLOCK_MONOTONIC CLOCK_MONOTONIC
#endif

#ifndef __GEN_FUNCTIONS
#define __GEN_FUNCTIONS
Expand Down Expand Up @@ -304,7 +309,7 @@ static void clock_gettime(int clk_id, struct timespec *tp) {

inline unsigned long long monotonic_time() {
struct timespec ts;
clock_gettime(CLOCK_MONOTONIC, &ts);
clock_gettime(PROXYSQL_CLOCK_MONOTONIC, &ts);
return (((unsigned long long) ts.tv_sec) * 1000000) + (ts.tv_nsec / 1000);
}

Expand Down
8 changes: 4 additions & 4 deletions lib/MySQL_Session.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3322,7 +3322,7 @@ void MySQL_Session::handler___status_WAITING_CLIENT_DATA___STATE_SLEEP___MYSQL_C
l_free(pkt.size,pkt.ptr);
client_myds->DSS=STATE_SLEEP;
status=WAITING_CLIENT_DATA;
CurrentQuery.end_time=thread->curtime;
CurrentQuery.set_end_time(thread->curtime);
CurrentQuery.end();
} else {
mybe=find_or_create_backend(current_hostgroup);
Expand Down Expand Up @@ -7187,7 +7187,7 @@ void MySQL_Session::handler___client_DSS_QUERY_SENT___server_DSS_NOT_INITIALIZED
#ifdef STRESSTESTPOOL_MEASURE
timespec begint;
timespec endt;
clock_gettime(CLOCK_MONOTONIC,&begint);
clock_gettime(PROXYSQL_CLOCK_MONOTONIC, &begint);
#endif // STRESSTESTPOOL_MEASURE
for (unsigned int loops=0; loops < NUM_SLOW_LOOPS; loops++) {
#endif // STRESSTEST_POOL
Expand All @@ -7213,7 +7213,7 @@ void MySQL_Session::handler___client_DSS_QUERY_SENT___server_DSS_NOT_INITIALIZED
}
#ifdef STRESSTEST_POOL
#ifdef STRESSTESTPOOL_MEASURE
clock_gettime(CLOCK_MONOTONIC,&endt);
clock_gettime(PROXYSQL_CLOCK_MONOTONIC, &endt);
thread->status_variables.query_processor_time=thread->status_variables.query_processor_time +
(endt.tv_sec*1000000000+endt.tv_nsec) -
(begint.tv_sec*1000000000+begint.tv_nsec);
Expand Down Expand Up @@ -7527,7 +7527,7 @@ unsigned long long MySQL_Session::IdleTime() {
void MySQL_Session::LogQuery(MySQL_Data_Stream *myds, const unsigned int myerrno, const char * errmsg) {
// we need to access statistics before calling CurrentQuery.end()
// so we track the time here
CurrentQuery.end_time=thread->curtime;
CurrentQuery.set_end_time(thread->curtime);

if (qpo) {
if (qpo->log==1) {
Expand Down
6 changes: 3 additions & 3 deletions lib/PgSQL_Session.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4573,7 +4573,7 @@ void PgSQL_Session::handler___client_DSS_QUERY_SENT___server_DSS_NOT_INITIALIZED
#ifdef STRESSTESTPOOL_MEASURE
timespec begint;
timespec endt;
clock_gettime(CLOCK_MONOTONIC, &begint);
clock_gettime(PROXYSQL_CLOCK_MONOTONIC, &begint);
#endif // STRESSTESTPOOL_MEASURE
for (unsigned int loops = 0; loops < NUM_SLOW_LOOPS; loops++) {
#endif // STRESSTEST_POOL
Expand Down Expand Up @@ -4601,7 +4601,7 @@ void PgSQL_Session::handler___client_DSS_QUERY_SENT___server_DSS_NOT_INITIALIZED
}
#ifdef STRESSTEST_POOL
#ifdef STRESSTESTPOOL_MEASURE
clock_gettime(CLOCK_MONOTONIC, &endt);
clock_gettime(PROXYSQL_CLOCK_MONOTONIC, &endt);
thread->status_variables.query_processor_time = thread->status_variables.query_processor_time +
(endt.tv_sec * 1000000000 + endt.tv_nsec) -
(begint.tv_sec * 1000000000 + begint.tv_nsec);
Expand Down Expand Up @@ -4885,7 +4885,7 @@ unsigned long long PgSQL_Session::IdleTime() {
void PgSQL_Session::LogQuery(PgSQL_Data_Stream* myds) {
// we need to access statistics before calling CurrentQuery.end()
// so we track the time here
CurrentQuery.end_time = thread->curtime;
CurrentQuery.set_end_time(thread->curtime);

if (qpo) {
if (qpo->log == 1) {
Expand Down
2 changes: 1 addition & 1 deletion test/tap/tap/utils.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -475,7 +475,7 @@ int create_table_test_sbtest1(int num_rows, MYSQL *mysql) {

unsigned long long monotonic_time() {
struct timespec ts;
clock_gettime(CLOCK_MONOTONIC, &ts);
clock_gettime(PROXYSQL_CLOCK_MONOTONIC, &ts);
return (((unsigned long long) ts.tv_sec) * 1000000) + (ts.tv_nsec / 1000);
}

Expand Down
7 changes: 7 additions & 0 deletions test/tap/tap/utils.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include <fstream>
#include <unistd.h>
#include <utility>
#include <time.h>

#include "curl/curl.h"
#include "mysql.h"
Expand All @@ -18,6 +19,12 @@
#include "command_line.h"
#include "mysql.h"

#ifdef CLOCK_MONOTONIC_RAW
#define PROXYSQL_CLOCK_MONOTONIC CLOCK_MONOTONIC_RAW
#else
#define PROXYSQL_CLOCK_MONOTONIC CLOCK_MONOTONIC
#endif
Comment on lines +22 to +26
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The definition of PROXYSQL_CLOCK_MONOTONIC is duplicated from include/gen_utils.h. To avoid this duplication, consider moving this macro to a more specific, shared header file that can be included by both gen_utils.h and this file. Alternatively, if dependencies allow, test/tap/tap/utils.h could include include/gen_utils.h directly.


template <typename T>
using rc_t = std::pair<int,T>;

Expand Down
Loading