You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Jean-Daniel Cryans (Code Review)" <ge...@cloudera.org> on 2016/12/09 23:39:07 UTC

[kudu-CR](branch-1.2.x) csd: two minor changes

Jean-Daniel Cryans has uploaded a new change for review.

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

Change subject: csd: two minor changes
......................................................................

csd: two minor changes

Revert the memory limit scaleFactor to the default value (1.0). 1.3 is
recommended for Java processes; not sure why Kudu was using that.

Convert the core dump directory to two separate role-level parameters. I
relented and did this once I saw that Impala also defines its core dump
directory in this way. In testing, the conversion was fine when the
service-level parameter wasn't set. When it was set, I saw that the
overridden value carried over to the roles even after the conversion
(good!). However, attempting to remove the so-called "role" overrides in the
UI yielded an error (bad!).

To work around this, I changed the names of the parameters (i.e. I prepended
either 'master' or 'tserver' to the names). This means any old value is lost
during an upgrade to the new CSD (a little bad), but that's better than not
being able to remove the value altogether.

Change-Id: I74b2fcb69fde9e0dbb232dba78db53cc1543b489
Reviewed-on: http://gerrit.cloudera.org:8080/5435
Tested-by: Kudu Jenkins
Reviewed-by: Jean-Daniel Cryans <jd...@apache.org>
(cherry picked from commit 251860b36fae96807c625f54d96c67e578685ab4)
---
M java/kudu-csd/src/descriptor/service.sdl
1 file changed, 23 insertions(+), 15 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I74b2fcb69fde9e0dbb232dba78db53cc1543b489
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: branch-1.2.x
Gerrit-Owner: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>

[kudu-CR](branch-1.2.x) csd: two minor changes

Posted by "Jean-Daniel Cryans (Code Review)" <ge...@cloudera.org>.
Jean-Daniel Cryans has posted comments on this change.

Change subject: csd: two minor changes
......................................................................


Patch Set 1: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I74b2fcb69fde9e0dbb232dba78db53cc1543b489
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: branch-1.2.x
Gerrit-Owner: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No

[kudu-CR](branch-1.2.x) csd: two minor changes

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

Change subject: csd: two minor changes
......................................................................


csd: two minor changes

Revert the memory limit scaleFactor to the default value (1.0). 1.3 is
recommended for Java processes; not sure why Kudu was using that.

Convert the core dump directory to two separate role-level parameters. I
relented and did this once I saw that Impala also defines its core dump
directory in this way. In testing, the conversion was fine when the
service-level parameter wasn't set. When it was set, I saw that the
overridden value carried over to the roles even after the conversion
(good!). However, attempting to remove the so-called "role" overrides in the
UI yielded an error (bad!).

To work around this, I changed the names of the parameters (i.e. I prepended
either 'master' or 'tserver' to the names). This means any old value is lost
during an upgrade to the new CSD (a little bad), but that's better than not
being able to remove the value altogether.

Change-Id: I74b2fcb69fde9e0dbb232dba78db53cc1543b489
Reviewed-on: http://gerrit.cloudera.org:8080/5435
Tested-by: Kudu Jenkins
Reviewed-by: Jean-Daniel Cryans <jd...@apache.org>
(cherry picked from commit 251860b36fae96807c625f54d96c67e578685ab4)
Reviewed-on: http://gerrit.cloudera.org:8080/5450
---
M java/kudu-csd/src/descriptor/service.sdl
1 file changed, 23 insertions(+), 15 deletions(-)

Approvals:
  Jean-Daniel Cryans: Looks good to me, approved
  Kudu Jenkins: Verified



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I74b2fcb69fde9e0dbb232dba78db53cc1543b489
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: branch-1.2.x
Gerrit-Owner: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins