Add logging options for recent N queries #43

jigyasumxkkxr · 2024-12-11T13:13:57Z

This update introduces new query logging option:

--log-N-queries=N : Saves the last N queries to tmp/node.3306/step-1/thread-0/recent-.sql. Defaults to 5 if not specified.

Fallback: If no logging option is selected, the default is to log the last 5 queries.

--log-all-queries : Saves a full query log to tmp/node.3306/step-1/thread-0.sql.

How to Test:

Enable --log-all-queries and confirm the full log is saved.
Set --log-N-queries=10 and check if only the last 10 queries are logged.
Ensure default behavior logs the last 5 queries when no options are selected.

rahulmalik87

Did you test it. The option should work without --pquery . So basically it should log last N sql into a file. by command . pstress-ms --table=1 --threads=1 --seconds=1
The changes you implement will work with option --pquery the requirement is it should work without that mode. Thanks for your time. Please make it work with default option

rahulmalik87 · 2024-12-12T12:51:35Z

src/thread.cpp

-      general_log << "Unable to open logfile for client output " << cl.str()
-                  << ": " << std::strerror(errno) << std::endl;
-      return;
+    if (options->at(Option::LOG_N_QUERIES) && options->at(Option::LOG_N_QUERIES)->getInt() > 0) {


If user pass --log-n-query=0, It should not log any query.

jigyasumxkkxr · 2024-12-13T05:17:40Z

src/thread.cpp

+  size_t n_queries = 5;
+
+  if (options->at(Option::LOG_N_QUERIES) &&
+      options->at(Option::LOG_N_QUERIES)->getInt() >= 0) {


Logic Corrected for --log-N-queries=0.
Now it will save an empty log file in myParams.logdir i.e /tmp here.

jigyasumxkkxr · 2024-12-13T06:12:20Z

Testing Examples :

Command 1 :

./pstress-ms --socket=/var/run/mysqld/mysqld.sock --password=<password> --database=<testdb> --no-encryption --no-tbs --grammar-sql=0 --no-ddl --table=1 --threads=1 --seconds=3

Default case : will save recent 5 logs to /tmp/node.3306_step_1_thread-0_recent-5.sql
Defaults : step = 1 , default thread = 0. Recent logs = 5.

Command 2 :

./pstress-ms --log-N-queries=10 --socket=/var/run/mysqld/mysqld.sock --password=<password> --database=<testdb> --no-encryption --no-tbs --grammar-sql=0 --no-ddl --table=1 --threads=1 --seconds=3

Recent Query Case : will save recent 10 logs to /tmp/node.3306_step_1_thread-0_recent-10.sql
Defaults : step = 1 , default thread = 0.

Command 3 :

./pstress-ms --log-all-queries --socket=/var/run/mysqld/mysqld.sock --password=<password> --database=<testdb> --no-encryption --no-tbs --grammar-sql=0 --no-ddl --table=1 --threads=1 --seconds=3

All Query Case : will save all logs to /tmp/node.3306_step_1_thread-0.sql
Defaults : step = 1 , default thread = 0.

Command 4 :

./pstress-ms --log-all-queries --log-N-queries=10 --socket=/var/run/mysqld/mysqld.sock --password=<password> --database=<testdb> --no-encryption --no-tbs --grammar-sql=0 --no-ddl --table=1 --threads=1 --seconds=3

All Query Case : will save all logs to /tmp/node.3306_step_1_thread-0.sql
Defaults : step = 1 , default thread = 0. All query logs will be printed.

Testing Examples (--pquery) :

Command 5 :

./pstress-ms --pquery --socket=/var/run/mysqld/mysqld.sock --password=<password> --database=<testdb> --no-encryption --no-tbs --grammar-sql=0 --no-ddl --table=1 --threads=1 --seconds=3

All Query Case : will save all logs to /tmp/node.3306_step_1_thread-0_recent-pquery-5.sql
Defaults : step = 1 , default thread = 0. Recent logs = 5.

Command 6 :

./pstress-ms --log-N-queries=10 --pquery --socket=/var/run/mysqld/mysqld.sock --password=<password> --database=<testdb> --no-encryption --no-tbs --grammar-sql=0 --no-ddl --table=1 --threads=1 --seconds=3

All Query Case : will save all logs to /tmp/node.3306_step_1_thread-0_recent-pquery-10.sql
Defaults : step = 1 , default thread = 0.

Command 7 :

./pstress-ms --log-all-queries --pquery --socket=/var/run/mysqld/mysqld.sock --password=<password> --database=<testdb> --no-encryption --no-tbs --grammar-sql=0 --no-ddl --table=1 --threads=1 --seconds=3

All Query Case : will save all logs to /tmp/node.3306_step_1_thread-0-pquery.sql
Defaults : step = 1 , default thread = 0. All query logs will be printed.

Command 8 :

./pstress-ms --log-all-queries --log-N-queries=10 --pquery --socket=/var/run/mysqld/mysqld.sock --password=<password> --database=<testdb> --no-encryption --no-tbs --grammar-sql=0 --no-ddl --table=1 --threads=1 --seconds=3

All Query Case : will save all logs to /tmp/node.3306_step_1_thread-0-pquery.sql
Defaults : step = 1 , default thread = 0. All query logs will be printed.

Observation :

For these arguments, when the --pquery argument is present, the full logs file looks like:

F SELECT * FROM t1;
Error Table 'testdb.t1' doesn't exist

For N=1, changes in the log file can be observed when using --log-N-queries=1, and looks like:

Error Table 'testdb.t1' doesn't exist

rahulmalik87

Very clean implementation. Good work. Getting there.
The requirement is to log last N queries executed by pstress in thread.sql. Two important changes.

use the same file that write log with --log-all-queries i.e thread_log file.
Last N queries by each thread and not overall. so one hint that structure should be part of Thread class.

jigyasumxkkxr · 2024-12-19T13:56:17Z

.vscode/settings.json

Ignore this added automatically by vscode

please add it .gitignore file. so it doesn't get upload and remove it.

src/random_test.cpp

jigyasumxkkxr · 2024-12-19T13:58:44Z

src/random_test.hpp

@@ -192,9 +192,9 @@ struct Index {
 struct Thd1 {
  Thd1(int id, std::ofstream &tl, std::ofstream &ddl_l, std::ofstream &client_l,
       MYSQL *c, std::atomic<unsigned long long> &p,
-       std::atomic<unsigned long long> &f)
+       std::atomic<unsigned long long> &f,int log_N_count)


Adding New arg for Thd1 structure

jigyasumxkkxr · 2024-12-19T13:59:51Z

src/thread.cpp

-        ? (log_filename.str()  + ".sql")  // All queries log
-        : (log_filename.str() + "_recent" 
-          + "-" + std::to_string(n_queries) + ".sql");
+  std::string full_log_filename = log_filename.str()  + ".sql"


Setting single file name for logs.

rahulmalik87

Please address review comments.

rahulmalik87 · 2024-12-20T03:59:47Z

.vscode/settings.json

please add it .gitignore file. so it doesn't get upload and remove it.

src/random_test.cpp

src/thread.cpp

rahulmalik87 · 2024-12-21T03:20:42Z

src/random_test.cpp

@@ -2679,11 +2679,16 @@ void Table::Compare_between_engine(const std::string &sql, Thd1 *thd) {
  set_default();
 }

+  // Data structures for recent queries
+  static std::deque<std::string> recent_queries;


Is it used any where?

I have corrected the logic and removed it

rahulmalik87 · 2024-12-21T03:21:37Z

src/random_test.hpp

@@ -42,6 +43,10 @@ std::string rand_float(float upper, float lower = 0);
 std::string rand_double(double upper, double lower = 0);
 std::string rand_string(int upper, int lower = 2);

+// Introduction of new function to retrive deque of recent queries
+std::deque<std::string> get_recent_queries();


Isn't better to make it part of Thd class?

Hmm, Removed and it is already part of Thd class

rahulmalik87 · 2024-12-21T03:22:03Z

src/random_test.hpp

@@ -42,6 +43,10 @@ std::string rand_float(float upper, float lower = 0);
 std::string rand_double(double upper, double lower = 0);
 std::string rand_string(int upper, int lower = 2);

+// Introduction of new function to retrive deque of recent queries
+std::deque<std::string> get_recent_queries();
+extern std::mutex recent_queries_mutex;


recent_queries_mutex is used no where

rahulmalik87 · 2024-12-21T03:22:49Z

src/random_test.cpp

@@ -2797,6 +2810,10 @@ bool execute_sql(const std::string &sql, Thd1 *thd) {

  return (res == 0 ? 1 : 0);
 }
+// Retrive the formed deque
+std::deque<std::string> get_recent_queries() {


Seems like you duplicating the function?

Removed as its already present in Thd Class

rahulmalik87 · 2024-12-21T03:23:20Z

src/thread.cpp

@@ -9,6 +9,10 @@
 std::atomic_flag lock_metadata = ATOMIC_FLAG_INIT;
 std::atomic<bool> metadata_loaded(false);

+// Mutex for thread-safe logging
+std::mutex general_log_mutex;


It is not used anywhere.

rahulmalik87 · 2024-12-21T03:28:15Z

src/random_test.cpp

 bool execute_sql(const std::string &sql, Thd1 *thd) {
  auto query = sql.c_str();
  static auto log_all = opt_bool(LOG_ALL_QUERIES);
  static auto log_failed = opt_bool(LOG_FAILED_QUERIES);
  static auto log_success = opt_bool(LOG_SUCCEDED_QUERIES);
+  static auto log_N = opt_bool(LOG_N_QUERIES);
+  thd->max_recent_queries = options->at(Option::LOG_N_QUERIES)->getInt();


you are reassigning this variable . This is done as part of Thd class initialization as well

Yes, removed

rahulmalik87 · 2024-12-21T03:29:50Z

src/random_test.cpp

 bool execute_sql(const std::string &sql, Thd1 *thd) {
  auto query = sql.c_str();
  static auto log_all = opt_bool(LOG_ALL_QUERIES);
  static auto log_failed = opt_bool(LOG_FAILED_QUERIES);
  static auto log_success = opt_bool(LOG_SUCCEDED_QUERIES);
+  static auto log_N = opt_bool(LOG_N_QUERIES);


user would provide a value for --log-N-queries. So either a option is Bool or Int. Can't be both

Removed this line and code is giving correct query logs i have tested

rahulmalik87 · 2024-12-21T03:30:48Z

src/random_test.cpp

@@ -2716,6 +2721,10 @@ bool execute_sql(const std::string &sql, Thd1 *thd) {
  if (res != 0) { // query failed
    thd->failed_queries_total++;
    thd->max_con_fail_count++;
+    // Manage recent queries for failed queries
+    if (!log_N) {


the condition should be if(options->at(Option::LOG_N_QUERIES)->getInt()>0)

rahulmalik87 · 2024-12-21T03:32:47Z

src/thread.cpp

  /* connection can be changed if we thd->tryreconnect is called */
  conn = thd->conn;
  delete thd;

+


Remove this empty lines

rahulmalik87 · 2024-12-21T03:35:12Z

src/thread.cpp

@@ -155,11 +170,21 @@ void Node::workerThread(int number) {
        break;
      }
    }
+    logDeque = thd->get_recent_queries();


Will this work ? in the case of mode pquery. you are not saving query.

hmm, removed, this will not work and as usual give 1-2 queries even if i ran --log-all-queries

rahulmalik87 · 2025-01-10T11:53:43Z

src/random_test.cpp

+#ifdef DUCKDB
+    try {
+        // Create DuckDB connection
+        duckdb::DuckDB db(nullptr);  // In-memory database


For each SQL to execute , It create new in memory database. This is not what is required here.
We have to create in memory database just once. use connection based on --thread we have to open a connection against that.

rahulmalik87

Create a new pull request and not append with N queries. The start look good. compile tryreconnect only for mysql. disable kill query for option for DUCKDB

rahulmalik87 requested changes Dec 12, 2024

View reviewed changes

jigyasumxkkxr commented Dec 13, 2024

View reviewed changes

recent-N-queries-logging

fed18f7

jigyasumxkkxr force-pushed the recent-N-queries branch from e832120 to fed18f7 Compare December 18, 2024 08:11

rahulmalik87 requested changes Dec 19, 2024

View reviewed changes

thd-specific

bc2ecdc

jigyasumxkkxr commented Dec 19, 2024

View reviewed changes

src/random_test.cpp Outdated Show resolved Hide resolved

jigyasumxkkxr commented Dec 19, 2024

View reviewed changes

src/random_test.cpp Outdated Show resolved Hide resolved

jigyasumxkkxr commented Dec 19, 2024

View reviewed changes

src/random_test.cpp Outdated Show resolved Hide resolved

jigyasumxkkxr commented Dec 19, 2024

View reviewed changes

rahulmalik87 requested changes Dec 20, 2024

View reviewed changes

jigyasumxkkxr added 2 commits December 20, 2024 12:12

log-updated

4ac23b5

.vscode-removed

db09c6a

rahulmalik87 requested changes Dec 21, 2024

View reviewed changes

jigyasumxkkxr added 2 commits December 21, 2024 10:55

final-resolved

dddc2ae

DuckDB

75ddee6

rahulmalik87 reviewed Jan 10, 2025

View reviewed changes

rahulmalik87 requested changes Jan 10, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add logging options for recent N queries #43

Add logging options for recent N queries #43

jigyasumxkkxr commented Dec 11, 2024

rahulmalik87 left a comment

rahulmalik87 Dec 12, 2024

jigyasumxkkxr Dec 13, 2024

jigyasumxkkxr commented Dec 13, 2024

rahulmalik87 left a comment

jigyasumxkkxr Dec 19, 2024

rahulmalik87 Dec 20, 2024

jigyasumxkkxr Dec 19, 2024

jigyasumxkkxr Dec 19, 2024

rahulmalik87 left a comment

rahulmalik87 Dec 20, 2024

rahulmalik87 Dec 21, 2024

jigyasumxkkxr Dec 21, 2024

rahulmalik87 Dec 21, 2024

jigyasumxkkxr Dec 21, 2024

rahulmalik87 Dec 21, 2024

jigyasumxkkxr Dec 21, 2024

rahulmalik87 Dec 21, 2024

jigyasumxkkxr Dec 21, 2024

rahulmalik87 Dec 21, 2024

jigyasumxkkxr Dec 21, 2024

rahulmalik87 Dec 21, 2024

jigyasumxkkxr Dec 21, 2024

rahulmalik87 Dec 21, 2024

jigyasumxkkxr Dec 21, 2024

rahulmalik87 Dec 21, 2024

jigyasumxkkxr Dec 21, 2024

rahulmalik87 Dec 21, 2024

jigyasumxkkxr Dec 21, 2024

rahulmalik87 Dec 21, 2024

jigyasumxkkxr Dec 21, 2024 •

edited

Loading

rahulmalik87 Jan 10, 2025

rahulmalik87 left a comment

Add logging options for recent N queries #43

Are you sure you want to change the base?

Add logging options for recent N queries #43

Conversation

jigyasumxkkxr commented Dec 11, 2024

This update introduces new query logging option:

rahulmalik87 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jigyasumxkkxr commented Dec 13, 2024

Testing Examples :

Testing Examples (--pquery) :

Observation :

rahulmalik87 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rahulmalik87 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jigyasumxkkxr Dec 21, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rahulmalik87 left a comment

Choose a reason for hiding this comment

jigyasumxkkxr Dec 21, 2024 •

edited

Loading