Skip to content

cli: Fix number of args expected by debug gossip-values#24752

Merged
craig[bot] merged 1 commit into
cockroachdb:masterfrom
a-robinson:gossipvalargs
Apr 12, 2018
Merged

cli: Fix number of args expected by debug gossip-values#24752
craig[bot] merged 1 commit into
cockroachdb:masterfrom
a-robinson:gossipvalargs

Conversation

@a-robinson
Copy link
Copy Markdown
Contributor

@a-robinson a-robinson commented Apr 12, 2018

Fixes #24751

Release note: None

It looks like the problem is that my original usage string (gossip-values <directory>) was badly copied and pasted from other debug commands, and then assumed necessary by #24399 when in reality no positional argument is used by the command.

@a-robinson a-robinson requested a review from nvb April 12, 2018 20:15
@a-robinson a-robinson requested a review from a team as a code owner April 12, 2018 20:15
@cockroach-teamcity
Copy link
Copy Markdown
Member

This change is Reviewable

Comment thread pkg/cli/debug.go Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make this Args: cobra.NoArgs, so that we still have argument count validation.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@a-robinson
Copy link
Copy Markdown
Contributor Author

bors r+

craig Bot pushed a commit that referenced this pull request Apr 12, 2018
24529: distsql: collect tableReader stats and output to trace r=asubiotto a=asubiotto

This PR is split into two commits. The first commit lays the groundwork for stat collection code. The stats are modeled as a proto to be sent over the wire in the future (we will be sending these stats as metadata). The second commit modifies the tableReader to use this InputStatCollector to count the number of rows it reads. Example usage:

```
root@:26257/> SELECT COUNT(*) FROM test.xyz;
+-------+
| count |
+-------+
|     7 |
+-------+
(1 row)

Time: 257.249037ms

root@:26257/> SET tracing = ON;
SET

Time: 362.575µs

root@:26257/> SELECT x,y,z FROM test.xyz ORDER BY x;
+---+---+------+
| x | y |  z   |
+---+---+------+
| 1 | 1 | NULL |
| 1 | 1 |    2 |
| 1 | 1 |    2 |
| 1 | 2 |    1 |
| 2 | 2 |    3 |
| 4 | 5 |    6 |
| 4 | 1 |    6 |
+---+---+------+
(7 rows)

Time: 207.334037ms

root@:26257/> SHOW TRACE FOR SESSION;

... rest of trace elided ...

| 2018-04-05 20:53:01.571467+00:00 | 21s794ms540µs951ns | waiting for read lock                                                    | [n1,s1,r23/1:/Table/5{2-3}]                                |     | /cockroach.roachpb.Internal/Batch |   13 |
| 2018-04-05 20:53:01.777272+00:00 | 22s345µs429ns      | read completed                                                           | [n1,s1,r23/1:/Table/5{2-3}]                                |     | /cockroach.roachpb.Internal/Batch |   13 |
| 2018-04-05 20:53:01.777435+00:00 | 22s508µs797ns      | stat summary for tableReader input                                       | [client=[::1]:64047,user=root,n1,TableReader]              |     | table reader                      |   11 |
|                                  |                    |                                                                          |                                                            |     |                                   |      |
|                                  |                    |  rows read         7                                                     |                                                            |     |                                   |      |

... rest of trace elided ...
```
And then through jaeger:
<img width="1427" alt="screen shot 2018-04-05 at 4 53 55 pm" src="https://user-images.githubusercontent.com/10560359/38392141-740ae6d8-38f4-11e8-9406-2c167142eba6.png">

Note that we can't currently run `SHOW TRACE FOR <QUERY>` in distsql mode, which I hope to fix soon (#16562)

Any suggestions to improve the output welcome.

cc @knz @jordanlewis 

24715: opt: Fix flaky test r=andy-kimball a=andy-kimball

Update the TestNoMallocs to use the SucceedsSoon method in order
to repeatedly retry until zero memory allocations are detected.

Release note: None

24752: cli: Fix number of args expected by `debug gossip-values` r=a-robinson a=a-robinson

Fixes #24751

Release note: None

It looks like the problem is that my original usage string (`gossip-values <directory>`) was badly copied and pasted from other debug commands, and then assumed necessary by #24399 when in reality no positional argument is used by the command.

Co-authored-by: Alfonso Subiotto Marqués <alfonso@cockroachlabs.com>
Co-authored-by: Andrew Kimball <andyk@cockroachlabs.com>
Co-authored-by: Alex Robinson <alexdwanerobinson@gmail.com>
@craig
Copy link
Copy Markdown
Contributor

craig Bot commented Apr 12, 2018

Build succeeded

@craig craig Bot merged commit e7be480 into cockroachdb:master Apr 12, 2018
@a-robinson a-robinson deleted the gossipvalargs branch May 18, 2018 20:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants