You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "zhen.zhang (Code Review)" <ge...@cloudera.org> on 2017/01/18 03:29:15 UTC

[kudu-CR] docs: add notes for multi master migration

zhen.zhang has uploaded a new change for review.

  http://gerrit.cloudera.org:8080/5728

Change subject: docs: add notes for multi master migration
......................................................................

docs: add notes for multi master migration

Change-Id: I7e469bc23b650f3b99c785311acc665ac20f33ea
---
M docs/administration.adoc
1 file changed, 13 insertions(+), 1 deletion(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/28/5728/1
-- 
To view, visit http://gerrit.cloudera.org:8080/5728
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I7e469bc23b650f3b99c785311acc665ac20f33ea
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: zhen.zhang <zh...@xiaomi.com>

[kudu-CR] docs: add notes for multi master migration

Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change.

Change subject: docs: add notes for multi master migration
......................................................................


Patch Set 4: Code-Review+2

Thanks for being patient.

-- 
To view, visit http://gerrit.cloudera.org:8080/5728
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I7e469bc23b650f3b99c785311acc665ac20f33ea
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: zhen.zhang <zh...@xiaomi.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: zhen.zhang <zh...@xiaomi.com>
Gerrit-HasComments: No

[kudu-CR] docs: add notes for multi master migration

Posted by "zhen.zhang (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,

I'd like you to reexamine a change.  Please visit

    http://gerrit.cloudera.org:8080/5728

to look at the new patch set (#4).

Change subject: docs: add notes for multi master migration
......................................................................

docs: add notes for multi master migration

Change-Id: I7e469bc23b650f3b99c785311acc665ac20f33ea
---
M docs/administration.adoc
1 file changed, 17 insertions(+), 6 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/28/5728/4
-- 
To view, visit http://gerrit.cloudera.org:8080/5728
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7e469bc23b650f3b99c785311acc665ac20f33ea
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: zhen.zhang <zh...@xiaomi.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: zhen.zhang <zh...@xiaomi.com>

[kudu-CR] docs: add notes for multi master migration

Posted by "zhen.zhang (Code Review)" <ge...@cloudera.org>.
zhen.zhang has posted comments on this change.

Change subject: docs: add notes for multi master migration
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/5728/2/docs/administration.adoc
File docs/administration.adoc:

PS2, Line 233: except when dumping uuid
> For simplicity of documentation, can we omit this exception? There's nothin
I got exception using the dump tool yesterday when add fs_data_dirs. And I just tried the tool a moment ago and the exception is gone, maybe I make some typo yesterday... Anyway, will remove the exception.


-- 
To view, visit http://gerrit.cloudera.org:8080/5728
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I7e469bc23b650f3b99c785311acc665ac20f33ea
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: zhen.zhang <zh...@xiaomi.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: zhen.zhang <zh...@xiaomi.com>
Gerrit-HasComments: Yes

[kudu-CR] docs: add notes for multi master migration

Posted by "zhen.zhang (Code Review)" <ge...@cloudera.org>.
zhen.zhang has posted comments on this change.

Change subject: docs: add notes for multi master migration
......................................................................


Patch Set 1:

(2 comments)

I just installed "Asciidoctor.js Live Preview", it's very useful, thanks :)

http://gerrit.cloudera.org:8080/#/c/5728/1/docs/administration.adoc
File docs/administration.adoc:

Line 292: NOTE:If the existing master's `fs_data_dirs` parameter is configured, you should also add it as a parameter of this format tool.
> Just so I understand, you've configured your _master_ with multiple data di
We configured to write wal in ssd, and write data in hdd, so wal path and data path are not the same. And yes we set master's fs_data_dirs with multiple data directories, because master and ts use the same configuration template... I'll try to fix this when building new cluster in the future.


Line 323: 
> Nit: reformat this new paragraph so that it adheres to the line  wrapping g
Sorry I don't get what you mean, as the length of this paragraph is not beyond the dashed red vertical line, and I tries to the the length same with other paragraphs.


-- 
To view, visit http://gerrit.cloudera.org:8080/5728
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I7e469bc23b650f3b99c785311acc665ac20f33ea
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: zhen.zhang <zh...@xiaomi.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: zhen.zhang <zh...@xiaomi.com>
Gerrit-HasComments: Yes

[kudu-CR] docs: add notes for multi master migration

Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change.

Change subject: docs: add notes for multi master migration
......................................................................


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/5728/3/docs/administration.adoc
File docs/administration.adoc:

Line 293: +
I'm a little confused, because if we need this plus to continue the list, don't we also need one on L321? Or is this one unnecessary?


-- 
To view, visit http://gerrit.cloudera.org:8080/5728
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I7e469bc23b650f3b99c785311acc665ac20f33ea
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: zhen.zhang <zh...@xiaomi.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: zhen.zhang <zh...@xiaomi.com>
Gerrit-HasComments: Yes

[kudu-CR] docs: add notes for multi master migration

Posted by "zhen.zhang (Code Review)" <ge...@cloudera.org>.
zhen.zhang has posted comments on this change.

Change subject: docs: add notes for multi master migration
......................................................................


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/5728/3/docs/administration.adoc
File docs/administration.adoc:

Line 293: +
> I'm a little confused, because if we need this plus to continue the list, d
Actually I'm not familiar with the Asciidoc grammer, but there is a plus on L251 and L500, so I think maybe we also need one here.


-- 
To view, visit http://gerrit.cloudera.org:8080/5728
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I7e469bc23b650f3b99c785311acc665ac20f33ea
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: zhen.zhang <zh...@xiaomi.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: zhen.zhang <zh...@xiaomi.com>
Gerrit-HasComments: Yes

[kudu-CR] docs: add notes for multi master migration

Posted by "zhen.zhang (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,

I'd like you to reexamine a change.  Please visit

    http://gerrit.cloudera.org:8080/5728

to look at the new patch set (#2).

Change subject: docs: add notes for multi master migration
......................................................................

docs: add notes for multi master migration

Change-Id: I7e469bc23b650f3b99c785311acc665ac20f33ea
---
M docs/administration.adoc
1 file changed, 14 insertions(+), 5 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/28/5728/2
-- 
To view, visit http://gerrit.cloudera.org:8080/5728
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7e469bc23b650f3b99c785311acc665ac20f33ea
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: zhen.zhang <zh...@xiaomi.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: zhen.zhang <zh...@xiaomi.com>

[kudu-CR] docs: add notes for multi master migration

Posted by "zhen.zhang (Code Review)" <ge...@cloudera.org>.
zhen.zhang has posted comments on this change.

Change subject: docs: add notes for multi master migration
......................................................................


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/5728/3/docs/administration.adoc
File docs/administration.adoc:

Line 293: +
> Actually I'm not familiar with the Asciidoc grammer, but there is a plus on
According to the following size, I think L321 don't need a plus, and also this one is unnecessary. http://asciidoctor.org/docs/user-manual/#complex-list-content


-- 
To view, visit http://gerrit.cloudera.org:8080/5728
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I7e469bc23b650f3b99c785311acc665ac20f33ea
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: zhen.zhang <zh...@xiaomi.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: zhen.zhang <zh...@xiaomi.com>
Gerrit-HasComments: Yes

[kudu-CR] docs: add notes for multi master migration

Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change.

Change subject: docs: add notes for multi master migration
......................................................................


Patch Set 1:

(5 comments)

Thanks for the patch!

BTW, it's helpful to view the generated documentation as well as the markup, to verify that the markup works well. To do that I've got the "Asciidoctor.js Live Preview" Google Chrome extension installed, and then I just navigate to the file on my filesystem (i.e. file:///...) and it renders it as it would render on the website.

http://gerrit.cloudera.org:8080/#/c/5728/1/docs/administration.adoc
File docs/administration.adoc:

PS1, Line 229: * Identify and record the directory where the master's data lives. If using Kudu system packages,
             :   the default value is /var/lib/kudu/master, but it may be customized via the `fs_wal_dir`
             :   configuration parameter.
I think the note about fs_data_dirs should be added to this bullet point rather than to a subset of the command examples. Then, also note here that if you've set fs_data_dirs to something other than the value of fs_wal_dir, it should be explicitly included in every command below where fs_wal_dir is also included.


Line 292: NOTE:If the existing master's `fs_data_dirs` parameter is configured, you should also add it as a parameter of this format tool.
Just so I understand, you've configured your _master_ with multiple data directories? What was the motivation for that? Consistency with the tserver configuration?

I'm asking because the master only manages the singleton catalog tablet, and so it's incredibly unlikely to fill up a single disk let alone multiple disks.


Line 323: 
Nit: reformat this new paragraph so that it adheres to the line  wrapping guideline (the dashed red vertical line) used by gerrit.


PS1, Line 325: list of masters, both new and existing
Replace the highlighted section with "list of all of the masters"


PS1, Line 418: * Identify and record the directory where the master's data lives. If using Kudu system packages,
             :   the default value is /var/lib/kudu/master, but it may be customized via the `fs_wal_dir`
             :   configuration parameter.
The same change should be made for this workflow.


-- 
To view, visit http://gerrit.cloudera.org:8080/5728
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I7e469bc23b650f3b99c785311acc665ac20f33ea
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: zhen.zhang <zh...@xiaomi.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes

[kudu-CR] docs: add notes for multi master migration

Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change.

Change subject: docs: add notes for multi master migration
......................................................................


Patch Set 2:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/5728/1/docs/administration.adoc
File docs/administration.adoc:

Line 292: ----
> We configured to write wal in ssd, and write data in hdd, so wal path and d
Gotcha. There's nothing wrong with doing this, it's just that the master does so little IO that you might consider it to be a waste of an SSD.


PS1, Line 418: [source,bash]
             : ----
             : $ kudu fs dump uuid --fs_w
> The same change should be made for this workflow.
Thanks for making the change below (L433-437), but for completeness we should also do it here.


http://gerrit.cloudera.org:8080/#/c/5728/2/docs/administration.adoc
File docs/administration.adoc:

PS2, Line 233: except when dumping uuid
For simplicity of documentation, can we omit this exception? There's nothing wrong with specifying fs_data_dirs in "kudu fs dump uuid", is there?


-- 
To view, visit http://gerrit.cloudera.org:8080/5728
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I7e469bc23b650f3b99c785311acc665ac20f33ea
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: zhen.zhang <zh...@xiaomi.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: zhen.zhang <zh...@xiaomi.com>
Gerrit-HasComments: Yes

[kudu-CR] docs: add notes for multi master migration

Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has submitted this change and it was merged.

Change subject: docs: add notes for multi master migration
......................................................................


docs: add notes for multi master migration

Change-Id: I7e469bc23b650f3b99c785311acc665ac20f33ea
Reviewed-on: http://gerrit.cloudera.org:8080/5728
Tested-by: Kudu Jenkins
Reviewed-by: Adar Dembo <ad...@cloudera.com>
---
M docs/administration.adoc
1 file changed, 17 insertions(+), 6 deletions(-)

Approvals:
  Adar Dembo: Looks good to me, approved
  Kudu Jenkins: Verified



-- 
To view, visit http://gerrit.cloudera.org:8080/5728
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I7e469bc23b650f3b99c785311acc665ac20f33ea
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: zhen.zhang <zh...@xiaomi.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: zhen.zhang <zh...@xiaomi.com>

[kudu-CR] docs: add notes for multi master migration

Posted by "zhen.zhang (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,

I'd like you to reexamine a change.  Please visit

    http://gerrit.cloudera.org:8080/5728

to look at the new patch set (#3).

Change subject: docs: add notes for multi master migration
......................................................................

docs: add notes for multi master migration

Change-Id: I7e469bc23b650f3b99c785311acc665ac20f33ea
---
M docs/administration.adoc
1 file changed, 18 insertions(+), 7 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/28/5728/3
-- 
To view, visit http://gerrit.cloudera.org:8080/5728
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7e469bc23b650f3b99c785311acc665ac20f33ea
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: zhen.zhang <zh...@xiaomi.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: zhen.zhang <zh...@xiaomi.com>