cli/sql: delay writing the history file to the end#88272
Merged
craig[bot] merged 1 commit intocockroachdb:masterfrom Sep 20, 2022
Merged
cli/sql: delay writing the history file to the end#88272craig[bot] merged 1 commit intocockroachdb:masterfrom
craig[bot] merged 1 commit intocockroachdb:masterfrom
Conversation
Previously, the shell would write to the history file in-between each input. This was incurring noticeable delays if the history file was large. This patch fixes that by delaying the file write until the end. This is akin to the behavior of unix shell. There is no user-visible change except in the case of a crash of the shell itself, which at this point has become extremely rare. Release note: None
Member
DrewKimball
reviewed
Sep 20, 2022
Collaborator
DrewKimball
left a comment
There was a problem hiding this comment.
Thanks for doing this so quickly!
Reviewable status:
complete! 0 of 0 LGTMs obtained (waiting on @DrewKimball and @knz)
pkg/cli/clisqlshell/sql.go line 2181 at r1 (raw file):
// large and we don't want the excess I/O latency to be interleaved // in-between every command. c.ins.SetAutoSaveHistory(histFile, false)
Just to make sure, saving the history to the file doesn't flush the in-memory data right? So this change shouldn't increase memory usage.
Contributor
Author
yes correct. |
DrewKimball
approved these changes
Sep 20, 2022
Collaborator
DrewKimball
left a comment
There was a problem hiding this comment.
Reviewable status:
complete! 1 of 0 LGTMs obtained (waiting on @DrewKimball and @knz)
Contributor
Author
|
TFYR! bors r=DrewKimball |
Contributor
|
Build failed (retrying...): |
Contributor
|
Build succeeded: |
knz
added a commit
to knz/cockroach
that referenced
this pull request
Sep 22, 2022
In light of PR cockroachdb#88272, we don't need the limit to be as low as 1000. This commit increases it back to 10000, which is the common default in unix shells and the screen/tmux utilities. Release note: None
craig bot
pushed a commit
that referenced
this pull request
Sep 22, 2022
87808: norm: do not fold floor division with unit denominator r=msirek a=msirek Fixes #87605 The floor division operator `//` is like division but drops the fractional portion of the result. Normalization rule FoldDivOne rewrites `(column // 1)` as `(column)` which is incorrect if the data in `column` has fractional digits, as those digits should be dropped. The solution is to only fold `(column // 1)` when `column` is one of the integer types. Release note (bug fix): This patch fixes incorrect results from the floor division operator, `//`, when the numerator is non-constant and the denominator is the constant 1. 88082: *: audit operation names in new schema changer r=Xiang-Gu a=Xiang-Gu This PR made non-functional changes related to new schema changer ops: 1. Rename all status changing operation on Index and Column to comform to the pattern: `Make[StatusA][Index/Column][StatusB]` 2. Rename a few constant/field/function names to be consistent with the principle of "being explicit": 2.1. Rename operation name `XxxGcXxx` to `XxxGCXxx`; 2.2. Rename field name in operations `DescID` to `DescriptorID`; 2.3. Rename descpb.DELETE_AND_WRITE_ONLY to descpb.WRITE_ONLY; 2.4. Rename sql.RunningStatusDeleteAndWriteOnly to sql.RunningStatusWriteOnly 3. Seveal comments enhencement 4. Rename `DELETE_AND_WRITE_ONLY` to `WRITE_ONLY` in some files under docs/. Fixes #84583 Release note: None 88459: kv: fix spelling of "interceptor" in file name r=nvanbenschoten a=nvanbenschoten All other file names and code objects have the correct spelling. Release justification: None. Release note: None. 88468: kvserver/batcheval: ignore UseRangeTombstonesForPointDeletes knob sometimes r=ajwerner a=ajwerner We want to ignore the point deletion knob for cases where we're deleting data from system tables watched by rangefeeds. Not doing so can lead to missed updates. This is particularly critical for cluster settings. Fixes #87575 Fixes #87479 Fixes #88202 Release note: None 88475: workload/schemachange/mixed-version: avoid index visibility r=fqazi a=fqazi Fixes: #87768 Previously, we were generating statements with invisible indexes in mixed version environments, which could lead to statements generated not supported on older versions. To address this, this patch will prevent the generation of create table/ create index statements with invisible indexes. Release note: None 88476: multitenant: add TestTenants() helper to TestServer r=ajstorm a=msbutler This patch adds a helper method for accessing information about the tenant(s) that were probabilistically created with the TestServer. Release note: None 88478: cli/sql: increase maxHistEntries r=ajwerner a=knz As discussed this morning. In light of PR #88272, we don't need the limit to be as low as 1000. This commit increases it back to 10000, which is the common default in unix shells and the screen/tmux utilities. Co-authored-by: Mark Sirek <sirek@cockroachlabs.com> Co-authored-by: Xiang Gu <xiang@cockroachlabs.com> Co-authored-by: Nathan VanBenschoten <nvanbenschoten@gmail.com> Co-authored-by: Andrew Werner <awerner32@gmail.com> Co-authored-by: Faizan Qazi <faizan@cockroachlabs.com> Co-authored-by: Michael Butler <butler@cockroachlabs.com> Co-authored-by: Raphael 'kena' Poss <knz@thaumogen.net>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #54679.
Previously, the shell would write to the history file in-between each input.
This was incurring noticeable delays if the history file was large.
This patch fixes that by delaying the file write until the end. This is akin to the behavior of unix shell.
There is no user-visible change except in the case of a crash of the shell itself, which at this point has become extremely rare.
Release note: None