Skip to content

system.tables improvements (total_rows/total_bytes/storage_policy)#9919

Merged
alexey-milovidov merged 16 commits intoClickHouse:masterfrom
azat:system.tables
Mar 30, 2020
Merged

system.tables improvements (total_rows/total_bytes/storage_policy)#9919
alexey-milovidov merged 16 commits intoClickHouse:masterfrom
azat:system.tables

Conversation

@azat
Copy link
Member

@azat azat commented Mar 29, 2020 •
edited
Loading

Changelog category (leave one):

  • Improvement

Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
Add total_rows/total_bytes into the system.tables table.

Detailed description / Documentation draft:

This patch-set contains the following changes:

This differs from the system.parts since it adds rows/bytes for Buffer/Memory too, which can be used to track memory usage (since the memory that is used for Buffer/Memory rows does not displays anywhere AFAICS)

Please note, that total_rows includes rows of the underlying storage for Buffer engine (since it uses IStorage::totalRows() which used for SELECT count() FROM table trivial queries), while total_bytes does not (since otherwise the purpose of this column will be broken).

Also some issues which was found during implementing this patch set has been fixed to:

  • optimize_trivial_count_query with GROUP BY

@azat azat changed the title system.tables improvements system.tables improvements (total_rows/total_bytes/storage_policy) Mar 29, 2020
@alexey-milovidov
Copy link
Member

alexey-milovidov commented Mar 29, 2020

Thank you! total rows in system.tables is very useful!

alexey-milovidov
alexey-milovidov reviewed Mar 29, 2020
View reviewed changes
@azat
Copy link
Member Author

azat commented Mar 29, 2020

00341_squashing_insert_select2

This is due to incorrect detection of optimize_trivial_count_query, fixed too (can be moved into separate PR, since it is completely unrelated to this one, but it's up to you)

@alexey-milovidov
Copy link
Member

alexey-milovidov commented Mar 29, 2020

It's fine :)

@alexey-milovidov alexey-milovidov added the pr-improvement Pull request with some product improvements label Mar 29, 2020
@azat
Copy link
Member Author

azat commented Mar 29, 2020

Ok, PR description updated (to reflect this info at least).

@azat
Copy link
Member Author

azat commented Mar 29, 2020

Integration tests (asan) — failed:1, passed:0, error:0

==================== 2 passed, 2 warnings in 76.88 seconds =====================

Hm, interesting.

@alexey-milovidov
Copy link
Member

alexey-milovidov commented Mar 29, 2020

Minor:

2020-03-29 17:18:04 /build/obj-x86_64-linux-gnu/../dbms/src/Storages/StorageBuffer.cpp:730:27: error: invalid case style for local variable 'underlyingRows' [readability-identifier-naming,-warnings-as-errors]
2020-03-29 17:18:04     std::optional<UInt64> underlyingRows;
2020-03-29 17:18:04                           ^~~~~~~~~~~~~~
2020-03-29 17:18:04                           underlying_rows

@azat
Copy link
Member Author

azat commented Mar 29, 2020

readability-identifier-naming

Interesting, why it does not report the same for totalBytes and totalRows

@azat
Copy link
Member Author

azat commented Mar 30, 2020 •
edited
Loading

So the following tests fails now, and AFAICS all of them are either flacky or fails in upstream/master too:

  • Integration tests -- fails in upstream/master too
  • Functional stateless tests (ubsan) — fail: 4 -- fails in upstream/master too (all 4 matched)
  • 01098_temporary_and_external_tables -- looks "flacky", since it does not fails in previous version of this patch (and it passed locally)
  • Not sure about performance tests (at least some of unstability looks like due to load on the worker)

@alexey-milovidov alexey-milovidov merged commit 9436a13 into ClickHouse:master Mar 30, 2020
@azat azat deleted the system.tables branch March 30, 2020 11:46
@azat azat mentioned this pull request May 22, 2020
Closed
@ka1bi4 ka1bi4 mentioned this pull request Jun 23, 2020
Merged
@azat azat mentioned this pull request Oct 23, 2020
Merged
@alexey-milovidov alexey-milovidov mentioned this pull request Aug 25, 2022
Closed
@alexey-milovidov
Copy link
Member

alexey-milovidov commented Aug 25, 2022

A bug has been found: #40637

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@alexey-milovidov alexey-milovidov alexey-milovidov approved these changes

Assignees

No one assigned

Labels

pr-improvement Pull request with some product improvements

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@azat @alexey-milovidov @qoega