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

release-2.1: sql: prevent a crash and add telemetry for an assertion #32742

Merged
merged 1 commit into from
Nov 30, 2018

Conversation

knz
Copy link
Contributor

@knz knz commented Nov 30, 2018

Backport 1/1 commits from #32713.

/cc @cockroachdb/release


Informs #32517.

@knz knz requested a review from jordanlewis November 30, 2018 12:14
@knz knz requested review from andreimatei and a team November 30, 2018 12:14
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@andreimatei
Copy link
Contributor

LGTM

We got reports in the wild that CockroachDB crashes upon SHOW
SESSIONS/QUERIES due to an invalid session ID in a ListSessions
response.

An invalid session ID should never happen; also conceptually it makes
no sense because if a session exists (insofar that it gets reported by
Listsessions) it must hav ean ID.

However I was not able to find how an invalid session ID could be
created, if at all. So the root cause of the problem remains
uncovered.

In the meantime, this patch prevents a crash when encountering an
invalid session ID. nil entries are transformed to SQL NULLs, and
non-nil entries with a non-16 size are reported as `<invalid>`. Also
the values are recorded and reported in telemetry.

Release note (bug fix): CockroachDB does not longer crash when
encountering an internal error related to invalid entries in the
output of SHOW SESSIONs.
@knz knz force-pushed the backport2.1-32713 branch from 5d63271 to 3460b0a Compare November 30, 2018 18:28
@knz knz merged commit ef0423b into cockroachdb:release-2.1 Nov 30, 2018
@knz knz deleted the backport2.1-32713 branch November 30, 2018 19:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants