shatteredsilicon / ssm-submodules

GNU Affero General Public License v3.0
1 stars 2 forks source link

Tuning Advisor Adjustments #301

Closed gordan-bobic closed 2 months ago

gordan-bobic commented 2 months ago

min: 48Mx2 (2 log files, old default) - never recommend less than this 96MB total

If at max it is <= 40% full, advise halving to next lower 2^n

If it is maxed out (fill > 90%, I think the currrent check is fill > 75%) , advise increase to next 2^n

gordan-bobic commented 2 months ago

I'm thinking that the per-thread memory usage into separate graphs.

1) Maximum Probable Connection Memory Usage - same as current block graph, minus max_allowed_packet, tmp_table_size, bulk_insert_buffer_size, read_rnd_buffer_size and read_buffer_size

2) Non-InnoDB Maximum Connection Memory Usage - bulk_insert_buffer_size, read_rnd_buffer_size and read_buffer_size - these are MyISAM only which isn't really heavily used any more.

3) Theoretically Possible Other Connection Memory Usage - max_allowed_packet, tmp_table_size - these are only used under specific circumstances which won't regularly materialise on most systems, and tmp_table_size isn't really limited, you can have thousands in any one session if you really want to.

And split the Theoretical Memory Usage graph and the Memory Usage Summary graphs in the same way to give a clearer overview.

oblitorum commented 2 months ago

Put up a warning at the top if there is less than 24 hours of worth of data. Or a permanent note at the top (like the one that says that if graphs are missing, to set innodb_monitor_enable=all on innodb advanced metrics dashboard) that says that the advice won't be reliable if there isn't at least 24 hours of telemetry data captured.

I added a permanent note to my local depolyment, let me know if it's good enough.

image

gordan-bobic commented 2 months ago

It would be better to word it as: "Tuning advice on this dashboard is unlikely to be reliable until at least 24 hours of telemetry data has accumulated and you zoom out to at least 24 hours." Also, please make it left-aligned rather than centered.

oblitorum commented 2 months ago

Then it should look like this

image

gordan-bobic commented 2 months ago

Can you align it to top rather than bottom, or shrink the vertical size of the box?

oblitorum commented 2 months ago

Here is the thing, there are 2 lines, the top line is the buildin Title line, with a drop down switch, the bottom line is the Body line image

This Grafana text panel is not very customizable, I can only align it to top and keep the Body line (like the one on innodb advanced metrics dashboard) image

Or set it as title so we can remove the bottom Body line, but it will be centered image

gordan-bobic commented 2 months ago

OK, centered it is then, that seems to match other similar cases on other dashboards.

oblitorum commented 2 months ago

innodb_log_file_size - this needs a note saying that it refers to total size of redo logs, which would be (innodb_log_file_size x innodb_log_files_in_group).

Is this note good enough? image

innodb_buffer_pool_size should be adjustable downward as well. Default is 128M, never advise less than that, but if our total InnoDB index size less than 40% of the buffer pool size, advise reducing it to the current size rounded up to the next 2^n. Or in other words, make adjustment advice work downward toward 128M minimum.

Note that current innodb_buffer_pool_size does advise tuning downward, the tuning rule is - always advise to the next multiple of innodb_buffer_pool_chunk_size x innodb_buffer_pool_instances of current innodb index size. e.g. innodb_buffer_pool_chunk_size x innodb_buffer_pool_instances is 128M, innodb index size is 30M, so the next mulpitle is 128M , and current innodb_buffer_pool_size is 256M, it will suggest downward to 128. And if the current innodb index size is 260M, it will suggest to next mulpitle 384M

image

If we're going to change it to only advises tuning downward when index size is less than 40% of the buffer pool size, shouldn't we also suggest a value that's a multple of innodb_buffer_pool_chunk_size x innodb_buffer_pool_instances ?

innodb_adaptive_hash_index - this is disabled on latest version of MariaDB by default. Put a note on it that we cannot test efficiency with it disabled, and that to be able to provide advice it would need to be enabled for a period to give the advisor some data to work with.

Is this note good enough? image

oblitorum commented 2 months ago

I'm thinking that the per-thread memory usage into separate graphs.

  • Maximum Probable Connection Memory Usage - same as current block graph, minus max_allowed_packet, tmp_table_size, bulk_insert_buffer_size, read_rnd_buffer_size and read_buffer_size
  • Non-InnoDB Maximum Connection Memory Usage - bulk_insert_buffer_size, read_rnd_buffer_size and read_buffer_size - these are MyISAM only which isn't really heavily used any more.
  • Theoretically Possible Other Connection Memory Usage - max_allowed_packet, tmp_table_size - these are only used under specific circumstances which won't regularly materialise on most systems, and tmp_table_size isn't really limited, you can have thousands in any one session if you really want to.

And split the Theoretical Memory Usage graph and the Memory Usage Summary graphs in the same way to give a clearer overview.

I adjusted locally, and it looks like this, sounds right? image

gordan-bobic commented 2 months ago

Memory graph changes look good. Yes, we should align to chunk X instances, but - note that instances is deprecated in recent MariaDB. I am also inclined to go up in powers of 2 because otherwise we are likely to end up in hysteresis state, that is why I proposed to propose down at under 40% and propose up at 90%, to give a 5-10% stability gap.

oblitorum commented 2 months ago

OK, then I'll make it go up/down in powers of 2 when it's above 90% / under 40%, and make sure the final value is aligned to a multiple of chunk_size x instances

gordan-bobic commented 2 months ago

Hmm... So next power of 2, round up to chunk x instances. And show a warning if either chunk or instances is not a power of 2, as that will upset the calculation.

Since instances is removed from 10.6, IIRC, make sure that it is assumed to be 1 if null.

oblitorum commented 2 months ago

And show a warning if either chunk or instances is not a power of 2, as that will upset the calculation.

Can we just make a permanent note like this?

image

Since instances is removed from 10.6, IIRC, make sure that it is assumed to be 1 if null.

Yeah, I made sure that.

gordan-bobic commented 2 months ago

I'm thinking we want a permanent warning, not a hover-over "i" warning. But now that I think about it, we don't need it - numbers will be weird, but it should work regardless of what chunks and instances is set to. So let's just not put that warning up (if chunk or instances isn't a power of 2).

oblitorum commented 2 months ago

OK, got it