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.

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

cli: More clearly differentiate between node listening and node advertising #27702

Closed
jseldess opened this issue Jul 18, 2018 · 0 comments
Closed
Labels
C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) S-3-ux-surprise Issue leaves users wondering whether CRDB is behaving properly. Likely to hurt reputation/adoption.

Comments

@jseldess
Copy link
Contributor

Currently, there's a confusing permutation of effects that can result from using --host and --advertise-host:

--host not specified --host specified
--advertise-host not specified Listen on all interfaces. Advertise the canonical host name to other nodes. Listen on the interface specified by --host. Advertise the value of --host to other nodes.
--advertise-host specified Listen on all interfaces. Advertise the value of --advertise-host to other nodes. Listen on the interface specified by --host. Advertise the value of --advertise-host to other nodes

Sometimes, --host is just for listening and --advertise-host is for advertising; other times, --host is for both listening and advertising.

From a UX perspective, it'd be ideal to clearly differentiate between listening and advertising. As @bdarnell has explained, changing --host is very backwards-incompatible, but perhaps there are other options (e.g., adding and emphasizing another flag like --listen-host while continuing to support --host).

cc: @knz, @bdarnell, @mberhault

@knz knz added C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) S-3-ux-surprise Issue leaves users wondering whether CRDB is behaving properly. Likely to hurt reputation/adoption. labels Jul 18, 2018
craig bot pushed a commit that referenced this issue Aug 7, 2018
27800: cli: sanitize the host/port command-line flags r=knz a=knz

Fixes  #27702.

This patch achieves the following:

- deprecates --host/--port in favor of --listen-addr/--listen-port for
  `cockroach start`
- produces a warning if neither --host/--listen nor --advertise are
  specified.
- deprecates --advertise-host in favor of --advertise-addr
- deprecates --http-host in favor of --http-addr
- suggests that "addresses" are also acceptable next to host names in
  the flag descriptions.
- suggests that the argument to --advertise must be routable
  everywhere.
- un-hides --advertise-port. This flag is actually useful when
  there is a NAT router with X:Y port forwarding.

Release note (cli change): the flags `--host` and `--port` to
`cockroach start` are deprecated in favor of `--listen-addr` and
`--listen-port`. They remain valid for other *client* commands.  The
flags `--advertise-host` and `--http-host` are deprecated in favor of
`--advertise-addr` and `--http-addr` and do not change meaning.

cc @jseldess 

28062: sql: do not use `rowsort` for queries that use ORDER BY r=knz a=knz

If a query uses ORDER BY, the test's expected output should be
stable, and the test should exercise that it is indeed stable and the
one expected given the ORDER BY constraint.

By improperly adding `rowsort` to a test that otherwises uses ORDER
BY, the value of the test is reduced as the quality of ORDER BY is not
tested any more.

This patch corrects this by removing `rowsort` from tests that also
use ORDER BY.

(There are a few tests that control the behavior of `rowsort` itself,
and these are left unchanged here.)

Release note: None

Co-authored-by: Raphael 'kena' Poss <[email protected]>
@craig craig bot closed this as completed in #27800 Aug 7, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) S-3-ux-surprise Issue leaves users wondering whether CRDB is behaving properly. Likely to hurt reputation/adoption.
Projects
None yet
Development

No branches or pull requests

2 participants