Skip to content

Conversation

bsrikanth-mariadb
Copy link
Contributor

@bsrikanth-mariadb bsrikanth-mariadb commented Sep 16, 2025

MDEV-33309: for update|delete analyze format=json doesn't show r_other_time_ms

Single-table UPDATE/DELETEs only show r_total_time_ms in top-level query block.
Replace it with r_table_time_ms and r_other_time_ms.

@bsrikanth-mariadb bsrikanth-mariadb force-pushed the 10.11-mdev-33309-no_r_other_time_ms_in_update_delete_trace branch from d032815 to 3ef1e17 Compare September 16, 2025 05:45
insert into t2 select * from t1;

--source include/analyze-format.inc
analyze format=json
Copy link
Member

Choose a reason for hiding this comment

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

Please add --echo # Top-level query block must have r_table_time_ms and r_other_time_ms

where t1.pk > (select max(a) from t2 where t2.pk+1 = t1.pk+1 ) - 10;

--source include/analyze-format.inc
analyze format=json
Copy link
Member

Choose a reason for hiding this comment

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

Please also add a comment like requested above.

@spetrunia
Copy link
Member

MDEV-33309: for update|delete analyze format=json doesn't show r_other_time_ms

Only r_total_time_ms is provided for query_block, in UPDATE|DELETE queries.
This PR replaces it with 2 metrics i.e. r_table_time_ms, and r_other_time_ms.

In commit comments, instead of "This PR does X" it is common to write "Do X"
Please change this comment to something line:

Single-table UPDATE/DELETEs only show r_total_time_ms in top-level query block.
Replace it with r_table_time_ms and r_other_time_ms.

@spetrunia
Copy link
Member

I was wondering where does the update time get billed to
Ran

 analyze format=json update t1 set a=a+1 where t1.pk > 994;

and paused it in ha_innobase::update() for a while.
The time got billed to the r_table_time_ms, good:

| {
  "query_optimization": {
    "r_total_time_ms": 0.814298762
  },
  "query_block": {
    "select_id": 1,
    "r_total_time_ms": 20056.984,
    "table": {
      "update": 1,
      "table_name": "t1",
      "access_type": "range",
      "possible_keys": ["PRIMARY"],
      "key": "PRIMARY",
      "key_length": "4",
      "used_key_parts": ["pk"],
      "rows": 6,
      "r_rows": 6,
      "r_filtered": 100,
      "r_table_time_ms": 20056.77929,
      "r_other_time_ms": 0.166702744,
      "r_engine_stats": {
        "pages_accessed": 11,
        "pages_updated": 15
      },
      "attached_condition": "t1.pk > 994"
    }
  }
} |

FK updates will be billed to the r_table_time_ms.
Triggers are run from the SQL layer and so will be billed to r_other_time_ms ...
This is ok, we just need to make a note in the docs.

@spetrunia spetrunia self-requested a review September 20, 2025 08:50
Copy link
Member

@spetrunia spetrunia left a comment

Choose a reason for hiding this comment

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

Please address the input above and then the patch is ok.

@bsrikanth-mariadb bsrikanth-mariadb force-pushed the 10.11-mdev-33309-no_r_other_time_ms_in_update_delete_trace branch from 3ef1e17 to 4208851 Compare September 22, 2025 04:07
…r_time_ms

Single-table UPDATE/DELETEs only show r_total_time_ms in top-level query block.
Replace it with r_table_time_ms and r_other_time_ms.
@bsrikanth-mariadb bsrikanth-mariadb force-pushed the 10.11-mdev-33309-no_r_other_time_ms_in_update_delete_trace branch 2 times, most recently from 6c3101c to 573d3ad Compare September 22, 2025 05:17
@bsrikanth-mariadb
Copy link
Contributor Author

Please address the input above and then the patch is ok.

Sure, addressed the comments.

@bsrikanth-mariadb bsrikanth-mariadb merged commit 573d3ad into 10.11 Sep 22, 2025
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

3 participants