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: incorrect formatting for index recommendations unused duration time #85222

Closed
THardy98 opened this issue Jul 28, 2022 · 0 comments · Fixed by #87529
Closed

sql: incorrect formatting for index recommendations unused duration time #85222

THardy98 opened this issue Jul 28, 2022 · 0 comments · Fixed by #87529
Assignees
@THardy98
Labels
C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior.

Comments

@THardy98
Copy link

THardy98 commented Jul 28, 2022 •
edited
Loading

Describe the problem

DB Console shows incorrect/weird unused duration time when displaying the reason to drop an unused index. The same logic is used for CC console as well so the issue is relevant to both platforms.

To Reproduce

Set the cluster setting sql.index_recommendation.drop_unused_duration to something small, like 0:0:10 (10 seconds). Wait 10 seconds without using a non-primary index. View the index on its corresponding index detail page and you will see incorrect duration formatting.

Expected behavior
Unused duration time should be displayed correctly.

Additional data / screenshots
Screen Shot 2022-07-28 at 9 28 46 AM
Bug shown on DB Console.

This is caused by:

func formatDuration(d time.Duration) string {
	days := d / (24 * time.Hour)
	hours := d % (24 * time.Hour)
	minutes := hours % time.Hour

	return fmt.Sprintf("%dd%dh%dm", days, hours/time.Hour, minutes)
}

which has been replicated on managed-service, causing the corresponding issue on CC console.

Jira issue: CRDB-18115
@THardy98 THardy98 added C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. T-sql-observability labels Jul 28, 2022
@THardy98 THardy98 mentioned this issue Aug 23, 2022
Merged
craig bot pushed a commit that referenced this issue Aug 25, 2022
@cucaroach @jbowens
Merge #85986 #86189 #86317
5c0af43 
85986: sql: make copy atomic by default and add optin to old behavior r=cucaroach a=cucaroach

Previously COPY under and implicit transaction would use auto committed
batches of 100 rows. This decision was made years ago when large txn
support wasn't what it is now. Today we handle large transactions better
so make the default bahavior atomic and provide a cluster setting to get
back the old behavior. Its possible in busy clusters with large COPY's
there may be contention and retries that make non-atomic copy desirable.

Fixes: #85887

Release note (sql change): COPY FROM operations are now atomic by
default instead of being segmented into 100 row transactions. Setting
the sql.copyfrom.atomic cluster setting to false to get the old
behavior.

Release justification: low risk high benefit correctness fix to existing
functionality

86189: storage/metamorphic: add TestCompareFiles r=nicktrav a=jbowens

Add a new MVCC metamorphic test entrypoint, TestCompareFiles, that takes a
check file through `--check` and one or more output.metas through
`--compare-files`. The test runs the configurations specified through
`--compare-files` against the specified `--check` file, parsing out the encoded
Pebble options.

Release note: None
Release justification: Non-production code changes

86317: ui: introduce schema insights page on db-console r=THardy98 a=THardy98

Resolves: #83828, [#83829](#83829)

This change introduces the schema insights page to the DB-Console.
Schema insights are fetched from `schemaInsightsApi` using the SQL-over
HTTP API and the corresponding component `schemaInsightsView` is
available from cluster-ui for future use in the CC console. The schema
insights page display a table of schema insights - currently different
types of index recommendations (i.e. drop/create/replace index
recommendations), with the intention to add different types of schema
insights in the future. Each schema insight row offers an actionable
button, offering the user the ability to execute the corresponding SQL
query that realizes their schema insight. Filters are available to
filter by database and the schema insight type, as well as search.

~~Loom: https://www.loom.com/share/29ac973730614968893c179f4974fc61~~
Updated Loom: https://www.loom.com/share/ee36842fa9594c8888523d9ce41e1607
(this demo shows a known bug in the duration formatting on the index details page when the duration is set to <1 hour, #85222)

Release note (ui change): Added new Schema Insights page to DB Console.
The Schema Insights page displays a table of schema insights - currently
different types of index recommendations (i.e. drop/create/replace index
recommendations). Each schema insight row offers the user the ability to
execute the corresponding SQL query that realizes their schema insight
via a clickable button. Filters are available to filter the surfaced
schema insights by database and insight type, as well as search.

Release justification: low risk, high benefit changes to existing
functionality

Co-authored-by: Tommy Reilly <[email protected]>
Co-authored-by: Jackson Owens <[email protected]>
Co-authored-by: Thomas Hardy <[email protected]>
@maryliag maryliag self-assigned this Aug 26, 2022
@THardy98 THardy98 self-assigned this Sep 1, 2022
@maryliag maryliag removed their assignment Sep 6, 2022
THardy98 pushed a commit to THardy98/cockroach that referenced this issue Sep 7, 2022
sql: fix unused duration formatting for index recommendations
8efaf63 
Addresses: cockroachdb#85222 for DB Console

Previously, the index details page would not display unused index
durations correctly for short durations (<1 hour).  This change corrects
the formatting logic to display these durations correctly, and in a more
readable format.

Release justification: low risk, high benefit changes to existing
functionality

Release note: None
@THardy98 THardy98 mentioned this issue Sep 7, 2022
Merged
THardy98 pushed a commit to THardy98/cockroach that referenced this issue Sep 8, 2022
sql: fix unused duration formatting for index recommendations
474034f 
Addresses: cockroachdb#85222 for DB Console

Previously, the index details page would not display unused index
durations correctly for short durations (<1 hour).  This change corrects
the formatting logic to display these durations correctly, and in a more
readable format.

Release justification: low risk, high benefit changes to existing
functionality

Release note: None
craig bot pushed a commit that referenced this issue Sep 8, 2022
@mgartner
Merge #87275 #87529
a3dee50 
87275: sql: support implicit record arg and return types in UDFs r=mgartner a=mgartner

#### opt: wrap last statement in a UDF with LIMIT 1

A UDF always returns a single row from the last statement in the UDF
(unless it is a set returning UDF, but we do not currently support
those). So, we can wrap the last statement in a `LIMIT 1` expression
which may allow additional optimizations to trigger.

Release justification: This is a minor change to a new feature.

Release note: None

#### opt: add implicit record types to opt test catalog

Release justification: This is a test-only change.

Release note: None

#### sql: support implicit record arg and return types in UDFs

Implicit record types are now supported as argument types and return
types of user-defined functions. For example:

    CREATE TABLE t (a INT PRIMARY KEY, b INT);
    CREATE FUNCTION ret_t() RETURNS t LANGUAGE SQL AS $$
      SELECT * FROM t
    $$;
    CREATE FUNCTION t_a(some_t t) RETURNS INT LANGUAGE SQL AS $$
      SELECT (some_t).a
    $$;

Fixes #86393

Release justification: This fixes a limitation of a newly added feature.

Release note: None

#### sql: propagate UDF errors correctly

This commit fixes a bug where an error that occurred during evaluation
of a UDF was not correctly propagated.

Release justification: This fixes a critical bug with a new feature.

Release note: None

#### sql: cast UDF result to function return type

The last statement of a UDF may have a resulting type, `R`, that does
not match the function's given return type, `F`. This is allowed if a
cast from `R` to `F` is valid in an implicit or assignment context.

This commit adds an assignment cast operator to the output column of the
final statement in the function body, if necessary.

Release justification: This fixes a minor bug with a new feature.

Release Note: None


87529: sql: fix unused duration formatting for index recommendations r=THardy98 a=THardy98

Fixes: #85222 for DB Console

Previously, the index details page would not display unused index durations correctly for short durations (<1 hour).  This change corrects the formatting logic to display these durations correctly, and in a more readable format.

Screenshot of new formatting:
![Screen Shot 2022-09-07 at 3 19 13 PM](https://user-images.githubusercontent.com/15315413/188960473-1014f827-be2f-423d-ac84-12bc5f6999de.png)

Release justification: low risk, high benefit changes to existing functionality

Release note: None

Co-authored-by: Marcus Gartner <[email protected]>
Co-authored-by: Thomas Hardy <[email protected]>
@craig craig bot closed this as completed in fa5929a Sep 8, 2022
blathers-crl bot pushed a commit that referenced this issue Sep 8, 2022
sql: fix unused duration formatting for index recommendations
51b2eb7 
Fixes: #85222 for DB Console

Previously, the index details page would not display unused index
durations correctly for short durations (<1 hour).  This change corrects
the formatting logic to display these durations correctly, and in a more
readable format.

Release justification: low risk, high benefit changes to existing
functionality

Release note: None
@blathers-crl blathers-crl bot mentioned this issue Sep 8, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@THardy98 THardy98

Labels
C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

sql: fix unused duration formatting for index recommendations THardy98/cockroach
2 participants
@maryliag @THardy98