cli: periodically flush in cockroach sql --format=csv
#28654
Labels
Milestone
Comments
danhhz
added
C-cleanup
|
I checked and currently it's only the csv/tsv and table writers that are buffered. You can already view your streaming data "live" with the other formatters, probably We can't change the table writer, that's something that we won't revisit any time soon (the thing needs all the rows to compute column widths, and we don't want to rewrite the tablewriter package). But doing the flush on the csv writer seems reasonable. |
craig bot
pushed a commit
that referenced
this issue
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>
craig bot
pushed a commit
that referenced
this issue
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
The "sinkless" version of changefeeds continuously streams back results to the user over pgwire. It'd be nice if
--format=csveventually printed every row that was returned (even if the stream stops returning results but doesn't close). Currently it doesn't because we never explicitly flush the writer. Flushing after every row would hurt perf too much but maybe we can do something with a timer.cc @knz who I mentioned this to. Perhaps
--format=tableshould get the same treatment. FYI definitely doesn't need to make 2.1 for cdc.The text was updated successfully, but these errors were encountered: