/cockroach

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

sql: report unimplemented vtables in telemetry #31332

Merged
merged 4 commits into from Oct 15, 2018

Conversation

Reviewers

@BramGruneir BramGruneir

@cockroachdb/core-prs core-prs

@cockroachdb/sql-execution-prs sql-execution-prs

@cockroachdb/sql-planning-prs sql-planning-prs

@dt dt

Assignees
No one assigned
Labels
None yet
Projects
  
Finished (milestone r2.1) in SQL Front-end, Lang & Semantics
Milestone
No milestone
3 participants
@knz @cockroach-teamcity @BramGruneir
@knz
Member

knz commented Oct 13, 2018 •
edited

Requested by @awoods187
First two commits from #31356.

Release note (sql change): CockroachDB will now collect references to
tables in information_schema and pg_catalog which are not yet
implemented, and report them as telemetry if statistics reporting is
enabled. This will help determine which features should be implemented
next for compatibility.

@knz knz requested a review from dt Oct 13, 2018

@knz knz added this to Triage in SQL Front-end, Lang & Semantics via automation Oct 13, 2018

@knz knz requested review from cockroachdb/core-prs as code owners Oct 13, 2018

@cockroach-teamcity

This comment has been minimized.

Show comment
Hide comment
@cockroach-teamcity

cockroach-teamcity Oct 13, 2018

Member

This change is Reviewable
Member

cockroach-teamcity commented Oct 13, 2018

This change is Reviewable

@knz knz referenced this pull request Oct 13, 2018

Open

sql: expose feature counters via crdb_internal.feature_usage #31334

@knz knz moved this from Triage to Current milestone in SQL Front-end, Lang & Semantics Oct 13, 2018

@knz
telemetry: minor optimization 
Release note: None
6f72a17

This was referenced Oct 15, 2018

Merged
Merged

knz added some commits Oct 13, 2018

@knz
sql: fold unimplemented/error telemetry into feature counters 
This was discussed with @dt: use the feature counter machinery
instead of using separate maps. This simplifies the code.

Release note: None
Loading status checks…
7d9920e
@knz
sql: report unimplemented vtables in telemetry 
Release note (sql change): CockroachDB will now collect references to
tables in `information_schema` and `pg_catalog` which are not yet
implemented, and report them as telemetry if statistics reporting is
enabled. This will help determine which features should be implemented
next for compatibility.
Loading status checks…
951e523

@knz knz requested a review from BramGruneir Oct 15, 2018

craig bot pushed a commit that referenced this pull request Oct 15, 2018

craig[bot] @knz
Merge #31333 #31356 
31333: pgerror: hint what to do upon encountering unimplemented errors r=knz a=knz

Requested by @awoods187 . Inspired by @tschottdorf 

Before:

```
root@127.0.0.1:28070/defaultdb> select * from information_schema.view_table_usage;
pq: virtual schema table not implemented: information_schema.view_table_usage
```

After:

```
root@127.0.0.1:28070/defaultdb> select * from information_schema.view_table_usage;
pq: virtual schema table not implemented: information_schema.view_table_usage
HINT: This feature is not yet implemented in CockroachDB.

Please check https://github.com/cockroachdb/cockroach/issues to check
whether this feature is already tracked. If you cannot find it there,
please report this error with example query at:

    https://github.com/cockroachdb/cockroach/issues/new/choose

If you would rather not post publicly, please contact us directly at:

    support@cockroachlabs.com

The Cockroach Labs team appreciates your feedback.
```

Release note: None

31356: sql,telemetry: fold unimplemented/error telemetry into feature counters r=knz a=knz

Forked off  #31332.


Co-authored-by: Raphael 'kena' Poss <knz@cockroachlabs.com>
Loading status checks…
b74a347
@BramGruneir

BramGruneir approved these changes Oct 15, 2018
View changes

:lgtm:

Only one nit.

Reviewed 1 of 1 files at r1, 7 of 7 files at r2, 5 of 5 files at r3.
Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (and 1 stale)

pkg/server/telemetry/features.go, line 97 at r2 (raw file):

// reporting.
func GetFeatureCounts() map[string]int32 {
	counters.RLock()

nit: defer runlock seems to be our style.

@knz
telemetry: use defer where appropriate 
Release note: None
Loading status checks…
eba46b3
@knz

knz reviewed Oct 15, 2018
View changes

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (and 1 stale)

pkg/server/telemetry/features.go, line 97 at r2 (raw file):

Previously, BramGruneir (Bram Gruneir) wrote…

nit: defer runlock seems to be our style.

done

@knz

This comment has been minimized.

Show comment
Hide comment
@knz

knz Oct 15, 2018

Member

thanks!

bors r+
Member

knz commented Oct 15, 2018

thanks!

bors r+

craig bot pushed a commit that referenced this pull request Oct 15, 2018

craig[bot] @knz
Merge #31332 
31332: sql: report unimplemented vtables in telemetry r=knz a=knz

Requested by @awoods187 
First two commits from  #31356.

Release note (sql change): CockroachDB will now collect references to
tables in `information_schema` and `pg_catalog` which are not yet
implemented, and report them as telemetry if statistics reporting is
enabled. This will help determine which features should be implemented
next for compatibility.

Co-authored-by: Raphael 'kena' Poss <knz@cockroachlabs.com>
Loading status checks…
93a48b3
@craig

This comment has been minimized.

Show comment
Hide comment
@craig

craig bot Oct 15, 2018

Build succeeded

craig bot commented Oct 15, 2018

Build succeeded

@craig craig bot merged commit eba46b3 into cockroachdb:master Oct 15, 2018
3 checks passed

3 checks passed

GitHub CI (Cockroach) TeamCity build finished
Details
bors Build succeeded
Details
license/cla Contributor License Agreement is signed.
Details

@knz knz deleted the knz:20181013-features branch Oct 15, 2018

@knz knz moved this from Current milestone to Finished (milestone r2.1) in SQL Front-end, Lang & Semantics Oct 15, 2018

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