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

make new schema changer logging less alarming #97922

Closed
massimocGH opened this issue Mar 2, 2023 · 0 comments · Fixed by #98112
Closed

make new schema changer logging less alarming #97922

massimocGH opened this issue Mar 2, 2023 · 0 comments · Fixed by #98112
Assignees
@fqazi
Labels
A-schema-changer-impl Related to the implementation of the new schema changer C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior.

Comments

@massimocGH
Copy link
Collaborator

massimocGH commented Mar 2, 2023 •
edited by cockroach-jira-scripts
Loading

Describe the problem

in 22.2.5 messages in the logs from the new schema changer seems to suggest a problem, while instead the true meaning of these messages is just to show that schema change is falling back to the legacy one:

I230302 14:40:35.717223 97101 sql/schemachanger/scbuild/internal/scbuildstmt/process.go:145 ⋮ [n1,client=127.0.0.1:52955,user=root] 309  failed building declarative schema change targets for GRANT with error: ‹*tree.Grant not implemented in the new schema changer›

I230302 14:41:31.380323 97101 sql/schemachanger/scbuild/internal/scbuildstmt/process.go:145 ⋮ [n1,client=127.0.0.1:52955,user=root] 320  failed building declarative schema change targets for GRANT with error: ‹*tree.Grant not implemented in the new schema changer›

in order to avoid confusion, can we rephrase these messages to something like:

declarative schema change targets for GRANT falling back to the legacy changer with message: ‹*tree.Grant not implemented in the new schema changer›

Environment:

  • CockroachDB version [22.2.5]

Jira issue: CRDB-24967
@massimocGH massimocGH added C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. A-schema-changer-impl Related to the implementation of the new schema changer labels Mar 2, 2023
@fqazi fqazi self-assigned this Mar 2, 2023
@fqazi fqazi mentioned this issue Mar 7, 2023
Merged
fqazi added a commit to fqazi/cockroach that referenced this issue Mar 7, 2023
@fqazi
sql/schmachanger: use more friendly language for fallback
49192c1 
Previously, the declarative schema changer used slightly
alarming language when falling back. To address this,
this patch removes the word failure and uses softer
langauge.

Fixes: cockroachdb#97922

Release note: None
craig bot pushed a commit that referenced this issue Mar 8, 2023
@jeffswenson @fqazi
Merge #97991 #98112
c705d32 
97991: server: move settingswatcher initialization to the start r=JeffSwenson a=JeffSwenson

Previously, the settingswatcher was one of the last services initialized during sql server start up. Now, it is one of the first services to start up. The settings do not have a well defined value until after settingwatcher initializes. Some settings, like cluster version, must have valid values during initialization.

Part of #94843

98112: sql/schmachanger: use more friendly language for fallback r=fqazi a=fqazi

Previously, the declarative schema changer used slightly
alarming language when falling back. To address this,
this patch removes the word failure and uses softer
language. Also, the logging level is reduced to remove
this message by default.

Fixes: #97922

Release note: None

Co-authored-by: Jeff <[email protected]>
Co-authored-by: Faizan Qazi <[email protected]>
@craig craig bot closed this as completed in 420a0aa Mar 8, 2023
@blathers-crl blathers-crl bot mentioned this issue Mar 8, 2023
Merged
blathers-crl bot pushed a commit that referenced this issue Mar 8, 2023
@fqazi
sql/schmachanger: use more friendly language for fallback
aa1ccaf 
Previously, the declarative schema changer used slightly
alarming language when falling back. To address this,
this patch removes the word failure and uses softer
language. Also the logging level is reduced to remove
this message by default.

Fixes: #97922

Release note: None
fqazi added a commit that referenced this issue Mar 10, 2023
@fqazi
sql/schmachanger: use more friendly language for fallback
ced3aad 
Previously, the declarative schema changer used slightly
alarming language when falling back. To address this,
this patch removes the word failure and uses softer
language. Also the logging level is reduced to remove
this message by default.

Fixes: #97922

Release note (bug fix): Previously the declarative schema changer
would emit alarming messages of the form: "failed building
declarative schema change targets for". These were non-severe
in nature and now disabled by defaulta.

Release note: None
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@fqazi fqazi

Labels
A-schema-changer-impl Related to the implementation of the new schema changer 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/schmachanger: use more friendly language for fallback fqazi/cockroach
2 participants
@fqazi @massimocGH