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.

Sign up for GitHub

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

Jump to bottom

sql: SHOW PARTITIONS doesn't show inherited constraints #40349

Closed
andreimatei opened this issue Aug 29, 2019 · 10 comments · Fixed by #40493
Closed

sql: SHOW PARTITIONS doesn't show inherited constraints #40349

andreimatei opened this issue Aug 29, 2019 · 10 comments · Fixed by #40493
Assignees
@rohany
Labels
A-partitioning C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) S-3-ux-surprise Issue leaves users wondering whether CRDB is behaving properly. Likely to hurt reputation/adoption.

Comments

@andreimatei
Copy link
Contributor

It's misleading that it doesn't. I think it should show them to you, and it should also tell you when they're inherited. No?
@rohany
Copy link
Contributor

rohany commented Aug 29, 2019

What do you mean by inherited constraints?

@andreimatei
Copy link
Contributor Author

andreimatei commented Aug 29, 2019 via email

@rohany
Copy link
Contributor

rohany commented Aug 29, 2019

Oh i see. It is misleading that it doesn't. We can take a look at this!

@awoods187 awoods187 added C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) S-3-ux-surprise Issue leaves users wondering whether CRDB is behaving properly. Likely to hurt reputation/adoption. labels Aug 30, 2019
@awoods187
Copy link
Contributor

Nice find @andreimatei . We should definitely do this.

@rohany
Copy link
Contributor

rohany commented Sep 2, 2019 •
edited
Loading

To do this correctly, it feels like we need to solve #40377 first.

@andreimatei andreimatei mentioned this issue Sep 3, 2019
Closed
@andreimatei
Copy link
Contributor Author

To do this correctly, it feels like we need to solve #40377 first.

Why? We have an inheritance today, so we should reflect that. Whether or not extend it seems orthogonal enough.
But perhaps more generally I'd agree with you that the data model is extremely awkward, so ideally we'd start over somehow.

@rohany
Copy link
Contributor

rohany commented Sep 3, 2019

Yeah, I meant to "correctly" do this. Displaying the current inheritance chain should be doable (though the fact that partitions that don't have constraints don't show up in the system.zones table complicates this).

@andreimatei
Copy link
Contributor Author

Well, inheritance is broken for sub-partitions, not for partitions in general. Regular partitions do inherit from their table/index.

@andreimatei
Copy link
Contributor Author

Btw, depending how you get your hands on a zone config (i.e. if you get it through calls like getZoneConfigForKey() and not directly from the system table), you get inherited fields in there, and you even get an InheritedConstraints field that tells you whether you've inherited crap.

@rohany
Copy link
Contributor

rohany commented Sep 3, 2019

Right now we are just joining against the crdb_internal.zones table which is why it is incorrect. The show commands are implemented through just straight translation into a new sql statement which is why this is annoying.

@rohany rohany mentioned this issue Sep 4, 2019
Merged
rohany added a commit to rohany/cockroach that referenced this issue Sep 9, 2019
@rohany
sql: Display inherited constraints in SHOW PARTITIONS
c796e79 
SHOW PARTITIONS now displays the inherited zone configuration
of the partitions in a separate column. To accomplish this, full
zone configuration information was added to the crdb_internal.partitions
table. The zone configuration information for each partitions was
retrieved using `GetZoneConfigInTxn` to avoid duplicating any
configuration inheritance logic.

Fixes cockroachdb#40349.

Release note (sql change): SHOW PARTITIONS now displays inherited zone
configurations.
rohany added a commit to rohany/cockroach that referenced this issue Sep 12, 2019
@rohany
sql: Display inherited constraints in SHOW PARTITIONS
fd66310 
SHOW PARTITIONS now displays the inherited zone configuration of the
partitions in a separate column. To accomplish this, the
crdb_internal.zones table now holds on to the inherited constraints of
each zone in a separate column. Additionally, the
crdb_internal.partitions table holds on to the zone_id and subzone_id of
the zone configuration the partition refers to. These id's correspond to
the zone configuration at the lowest point in that partitions
"inheritance chain".

Fixes cockroachdb#40349.

Release note (sql change): SHOW PARTITIONS now displays inherited zone
configurations.
rohany added a commit to rohany/cockroach that referenced this issue Sep 16, 2019
@rohany
sql: Display inherited constraints in SHOW PARTITIONS
b0745e5 
SHOW PARTITIONS now displays the inherited zone configuration of the
partitions in a separate column. To accomplish this, the
crdb_internal.zones table now holds on to the inherited constraints of
each zone in a separate column. Additionally, the
crdb_internal.partitions table holds on to the zone_id and subzone_id of
the zone configuration the partition refers to. These id's correspond to
the zone configuration at the lowest point in that partitions
"inheritance chain".

Release justification: Adds a low risk, good to have UX feature.

Fixes cockroachdb#40349.

Release note (sql change):
* SHOW PARTITIONS now displays inherited zone configurations.
* Adds the zone_id, subzone_id columns to crdb_internal.partitions,
which form a link to the corresponding zone config in
crdb_internal.zones which apply to the partitions.
* Rename the config_yaml and config_sql columns in crdb_internal.zones
to raw_config_yaml and raw_config_sql.
* Add the columns full_config_sql and full_config_yaml to the
crdb_internal.zones table which display the full/inherited zone
configuration.
craig bot pushed a commit that referenced this issue Sep 30, 2019
@rohany @dt
Merge #40493 #41138 #41196
d1f5ea4 
40493: sql: Display inherited constraints in SHOW PARTITIONS  r=andreimatei a=rohany

SHOW PARTITIONS now displays the inherited zone configuration of the
partitions in a separate column. To accomplish this, the
crdb_internal.zones table now holds on to the inherited constraints of
each zone in a separate column. Additionally, the
crdb_internal.partitions table holds on to the zone_id and subzone_id of
the zone configuration the partition refers to. These id's correspond to
the zone configuration at the lowest point in that partitions
"inheritance chain".

Release justification: Adds a low risk, good to have UX feature.

Fixes #40349.

Release note (sql change):
* SHOW PARTITIONS now displays inherited zone configurations.
* Adds the zone_id, subzone_id columns to crdb_internal.partitions,
which form a link to the corresponding zone config in
crdb_internal.zones which apply to the partitions.
* Rename the config_yaml, config_sql and config_proto columns in
crdb_internal.zones to raw_config_yaml, raw_config_sql,
raw_config_proto.
* Add the columns full_config_sql and full_config_yaml to the
crdb_internal.zones table which display the full/inherited zone
configuration.

41138: movr: Add stats collection to movr workload run r=danhhz a=rohany

This PR adds tracking stats for each kind of query in the movr workload
so that output is displayed from cockroach workload run. Additionally,
this refactors the movr workload to define the work as functions on a
worker struct. This hopefully will avoid a common gotcha of having
different workers sharing the same not threadsafe histograms object.

Release justification: low risk nice to have feature

Release note: None

41196: store,bulk: log when delaying AddSSTable, collect + log more timings in bulk-ingest r=dt a=dt

storage: log when AddSSTable requests are delayed

If the rate-limiting and back-pressure mechanisms kick in, they can dramatically delay requests in some cases.
However there is currently it can be unclear that this is happening and the system may simply appear slow.
Logging when requests are delayed by more than a second should help identify when this is the cause of slowness.

Release note: none.

Release justification: low-risk (logging only) change that could significantly help in diagnosing 'stuck' jobs based on logs (which often all we have to go on).

bulk: track and log more timings

This tracks and logs time spent in the various stages of ingestion - sorting, splitting and flushing.
This helps when trying to diagnose why a job is 'slow' or 'stuck'.

Release note: none.

Release justification: low-risk (logging only) changes that improve ability to diagnose problems.


Co-authored-by: Rohan Yadav <rohany@alumni.cmu.edu>
Co-authored-by: Rohan Yadav <rohany@cockroachlabs.com>
Co-authored-by: David Taylor <tinystatemachine@gmail.com>
@craig craig bot closed this as completed in a7a8fe6 Sep 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@rohany rohany

Labels
A-partitioning C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) S-3-ux-surprise Issue leaves users wondering whether CRDB is behaving properly. Likely to hurt reputation/adoption.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

sql: Display inherited constraints in SHOW PARTITIONS rohany/cockroach
3 participants
@andreimatei @rohany @awoods187