Skip to content
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.

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

Add log message when stacktrace cannot be obtained for thread #20996

Merged

Conversation

azat
Copy link
Collaborator

@azat azat commented Feb 20, 2021

Changelog category (leave one):

  • Not for changelog (changelog entry is not required)

This is to provide better diagnostics for 01051_system_stack_trace failure 1.

@robot-clickhouse robot-clickhouse added the pr-not-for-changelog This PR should not be mentioned in the changelog label Feb 20, 2021
Copy link
Member

@alexey-milovidov alexey-milovidov left a comment

Choose a reason for hiding this comment

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

Ok.

Caveats:

  1. If stack trace in the result is empty - it is already obvious that we cannot obtain it. Why do additional logging with no much details?
  2. If it cannot obtain stack trace - usually it cannot do it for all threads - so it will simply write ~1000 useless messages.

@alexey-milovidov alexey-milovidov self-assigned this Feb 20, 2021
Copy link
Member

@alexey-milovidov alexey-milovidov left a comment

Choose a reason for hiding this comment

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

Ok.

@alexey-milovidov alexey-milovidov merged commit 2ac5c4b into ClickHouse:master Feb 21, 2021
@azat
Copy link
Collaborator Author

azat commented Feb 21, 2021

If it cannot obtain stack trace - usually it cannot do it for all threads - so it will simply write ~1000 useless messages.

Actually it will not fail for all threads, look:

$ pigz -cd clickhouse-server\ \(5\).log.gz | fgrep -a 034edc75-91b1-4eba-a639-52389b6eaa6e
2021.02.20 12:14:53.707097 [ 74226 ] {034edc75-91b1-4eba-a639-52389b6eaa6e} <Debug> executeQuery: (from [::1]:37518, using production parser) (comment: /usr/share/clickhouse-test/queries/0_stateless/01051_system_stack_trace.sql) -- at least this query should be present
2021.02.20 12:14:53.711233 [ 74226 ] {034edc75-91b1-4eba-a639-52389b6eaa6e} <Trace> ContextAccess (default): Access granted: SELECT(query_id) ON system.stack_trace
2021.02.20 12:14:53.846474 [ 74226 ] {034edc75-91b1-4eba-a639-52389b6eaa6e} <Debug> StorageSystemStackTrace: Cannot obtain a stack trace for thread 1198
2021.02.20 12:14:53.895202 [ 74226 ] {034edc75-91b1-4eba-a639-52389b6eaa6e} <Trace> InterpreterSelectQuery: FetchColumns -> Complete
2021.02.20 12:14:53.899864 [ 308 ] {034edc75-91b1-4eba-a639-52389b6eaa6e} <Trace> AggregatingTransform: Aggregating
2021.02.20 12:14:53.899996 [ 308 ] {034edc75-91b1-4eba-a639-52389b6eaa6e} <Trace> Aggregator: Aggregation method: without_key
2021.02.20 12:14:53.900191 [ 308 ] {034edc75-91b1-4eba-a639-52389b6eaa6e} <Trace> AggregatingTransform: Aggregated. 1 to 1 rows (from 0.00 B) in 0.003031202 sec. (329.90213123374815 rows/sec., 0.00 B/sec.)
2021.02.20 12:14:53.900310 [ 308 ] {034edc75-91b1-4eba-a639-52389b6eaa6e} <Trace> Aggregator: Merging aggregated data
2021.02.20 12:14:53.902025 [ 74226 ] {034edc75-91b1-4eba-a639-52389b6eaa6e} <Information> executeQuery: Read 323 rows, 36.62 KiB in 0.194526252 sec., 1660 rows/sec., 188.23 KiB/sec.
2021.02.20 12:14:53.902283 [ 74226 ] {034edc75-91b1-4eba-a639-52389b6eaa6e} <Debug> MemoryTracker: Peak memory usage (for query): 0.00 B.

Fortunately thread 1198 wasn't the thread of the select * from system.stack_trace so the test had been passed.

So it seems that waiting 100ms is too small, and it should be made configurable (and increased in tests)

@azat azat deleted the system-stack-trace-diagnostics branch February 21, 2021 20:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr-not-for-changelog This PR should not be mentioned in the changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants