cli: periodically flush csv/tsv output #28688
Merged
Conversation
This file contains 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
|
This change is |
danhhz
reviewed
Aug 16, 2018
There was a problem hiding this comment.
Reviewable status:
complete! 0 of 0 LGTMs obtained (and 1 stale)
pkg/cli/format_table.go, line 341 at r1 (raw file):
}() cleanup := func() { r.stop <- struct{}{}
I hear the idiomatic way to do this is to close the channel
knz
force-pushed
the
20180816-flush-csv
branch
from
3742845
to
0dac237
Compare
knz
commented
Aug 31, 2018
There was a problem hiding this comment.
Reviewable status:
pkg/cli/format_table.go, line 341 at r1 (raw file):
Previously, danhhz (Daniel Harrison) wrote…
I hear the idiomatic way to do this is to close the channel
Done.
|
TFYR! bors r+ |
Build failed (retrying...) |
The "sinkless" version of changefeeds continuously streams back results to the user over pgwire. Prior to this patch, this data could not be consumed effectively with `cockroach sql` using the tsv/csv output, because the tsv/csv formatter buffers rows internally. This patch makes tsv/csv output in `cockroach sql` an effective way to consume changefeeds by ensuring an upper bound on the time rows stays buffered inside the formatter. The flush period is fixed to 5 seconds. For context, all the other formatters except for `table` are line-buffered and thus flush on every row. `table` is a world of its own which buffers *all* the rows until the query is complete, and that is unlikely to change any time soon, so this patch doesn't touch that either. Release note (cli change): The `csv` and `tsv` formats for `cockroach` commands that output result rows now buffer data for a maximum of 5 seconds. This makes it possible to e.g. view SQL changefeeds interactively with `cockroach sql` and `cockroach demo`.
knz
force-pushed
the
20180816-flush-csv
branch
from
0dac237
to
1dc7788
Compare
Canceled |
|
bors r+ |
craig bot
pushed a commit
that referenced
this pull request
Aug 31, 2018
28688: cli: periodically flush csv/tsv output r=knz a=knz Fixes #28654. The "sinkless" version of changefeeds continuously streams back results to the user over pgwire. Prior to this patch, this data could not be consumed effectively with `cockroach sql` using the tsv/csv output, because the tsv/csv formatter buffers rows internally. This patch makes tsv/csv output in `cockroach sql` an effective way to consume changefeeds by ensuring an upper bound on the time rows stays buffered inside the formatter. The flush period is fixed to 5 seconds. For context, all the other formatters except for `table` are line-buffered and thus flush on every row. `table` is a world of its own which buffers *all* the rows until the query is complete, and that is unlikely to change any time soon, so this patch doesn't touch that either. Release note (cli change): The `csv` and `tsv` formats for `cockroach` commands that output result rows now buffer data for a maximum of 5 seconds. This makes it possible to e.g. view SQL changefeeds interactively with `cockroach sql` and `cockroach demo`. 29446: cli: avoid deprecation warnings for deprecated flags r=knz a=knz Suggested/recommended by @a-robinson. The flags `--host`, `--advertise-host`, etc are now deprecated, but there is no cost in continuing to support them. Also users migrating from previous versions are not losing anything (or missing out) by continuing to use them. Forcing a warning to appear when they are used does not bring any tangible benefit and risks creating operator fatigue. So this patch removes the warning (but keeps the deprecated flags hidden, so that new users are guided to the new flags). Release note: None Co-authored-by: Raphael 'kena' Poss <knz@cockroachlabs.com>
Build succeeded |
craig bot
pushed a commit
that referenced
this pull request
Sep 1, 2018
29445: release-2.1: cli: periodically flush csv/tsv output r=knz a=knz Backport 1/1 commits from #28688. /cc @cockroachdb/release --- Fixes #28654. The "sinkless" version of changefeeds continuously streams back results to the user over pgwire. Prior to this patch, this data could not be consumed effectively with `cockroach sql` using the tsv/csv output, because the tsv/csv formatter buffers rows internally. This patch makes tsv/csv output in `cockroach sql` an effective way to consume changefeeds by ensuring an upper bound on the time rows stays buffered inside the formatter. The flush period is fixed to 5 seconds. For context, all the other formatters except for `table` are line-buffered and thus flush on every row. `table` is a world of its own which buffers *all* the rows until the query is complete, and that is unlikely to change any time soon, so this patch doesn't touch that either. Release note (cli change): The `csv` and `tsv` formats for `cockroach` commands that output result rows now buffer data for a maximum of 5 seconds. This makes it possible to e.g. view SQL changefeeds interactively with `cockroach sql` and `cockroach demo`. Co-authored-by: Raphael 'kena' Poss <knz@cockroachlabs.com>
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 #28654.
The "sinkless" version of changefeeds continuously streams back
results to the user over pgwire. Prior to this patch, this data could
not be consumed effectively with
cockroach sqlusing the tsv/csvoutput, because the tsv/csv formatter buffers rows internally.
This patch makes tsv/csv output in
cockroach sqlan effective way toconsume changefeeds by ensuring an upper bound on the time rows stays
buffered inside the formatter. The flush period is fixed to 5 seconds.
For context, all the other formatters except for
tableareline-buffered and thus flush on every row.
tableis a world of itsown which buffers all the rows until the query is complete, and that
is unlikely to change any time soon, so this patch doesn't touch that
either.
Release note (cli change): The
csvandtsvformats forcockroachcommands that output result rows now buffer data for a maximum of 5
seconds. This makes it possible to e.g. view SQL changefeeds
interactively with
cockroach sqlandcockroach demo.