Re: Review Request 49455: Optimized classpath scannig for upgrade check impelemtations
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/49455/#review140304 --- Ship it! Ship It! - Daniel Gergely On jún. 30, 2016, 4:15 du, Laszlo Puskas wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/49455/ > --- > > (Updated jún. 30, 2016, 4:15 du) > > > Review request for Ambari, Daniel Gergely, Sumit Mohanty, and Sebastian > Toader. > > > Bugs: AMBARI-17505 > https://issues.apache.org/jira/browse/AMBARI-17505 > > > Repository: ambari > > > Description > --- > > Problem: > During startup the ambari server scasns the classpath for finding components > to be bound in the IoC context. > When binding upgrade check implementations the full ambari package is scanned > that leads to prolonged startup time. > > Solution: > As upgrade check implementations reside in a dedicated package, the scanner > is modified to lookup them in this very package. > (on the local env this shortens the startup time by ~25 seconds) > > > Diffs > - > > > ambari-server/src/main/java/org/apache/ambari/server/controller/ControllerModule.java > e0bda13 > > Diff: https://reviews.apache.org/r/49455/diff/ > > > Testing > --- > > Unit tests running. > > > Thanks, > > Laszlo Puskas > >
Re: Review Request 49265: When querying host role command entities, the number of parameters should be limited
> On jún. 27, 2016, 5:36 du, Alejandro Fernandez wrote: > > ambari-server/src/main/java/org/apache/ambari/server/orm/dao/HostRoleCommandDAO.java, > > line 288 > > <https://reviews.apache.org/r/49265/diff/1/?file=1431054#file1431054line288> > > > > What's the root cause of more than 1k rows being returned? > > RU/EU? > > Daniel Gergely wrote: > The number of returned lines is irrelevant here. The problem is the > number of parameters in the prepared statement. The query above has "task_id > IN ?1" part and "?1" is replaced with a collection. If that collection has > many elements, the prepared statement looks like "task_id IN > (?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,...)". There is an upper limit > for the number of parameters on the sql server side. This was handled for > oracle only, but mssql has a limit as well, so it has no point to apply the > logic only for oracle, rather now it is configurable. > The number of parameteres is high when stage summaries are being created > and cache is empty (e.g.: because of an earlier host removal or an ambari > server restart). That case all tasks are in the collection, so in the case of > a 200 nodes cluster the number of tasks can be over 2200. (mssql rejected the > request, because the upper limit for number of parameteres in this case was > 2100) > > What does "RU/EU" mean? > > Sebastian Toader wrote: > RU - rolling upgrade, EU - express upgrade > > Daniel Gergely wrote: > I see. It is not related to upgrade process at all. > > Alejandro Fernandez wrote: > My point is that we need to know what actually made this request. > I understand why it is being segmented, but I'm curious if the caller is > inefficient and actually queries sequential task_ids, in which case we can > use task_id >= x and task_id <= y. It is not necessary sequential. The problem is with the design: the filtering is done on ambari-server side, not in the db. So what the method receives is a list of taskIds (not the information with which db filtering could be done). This method is called when creating a summary for stages (e.g. for the ui), so all the tasks are requested. In the case of a large cluster, the mentioned limit is hit. (it is 1000 for oracle, 2100 for mssql, 65536 for mysql, and unlimited for postgres) - Daniel --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/49265/#review139611 --- On jún. 27, 2016, 2:01 du, Daniel Gergely wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/49265/ > --- > > (Updated jún. 27, 2016, 2:01 du) > > > Review request for Ambari, Laszlo Puskas, Oliver Szabo, Sandor Magyari, Sumit > Mohanty, and Sebastian Toader. > > > Bugs: AMBARI-17449 > https://issues.apache.org/jira/browse/AMBARI-17449 > > > Repository: ambari > > > Description > --- > > In the case of a larger cluster, the number of host role commands can be high > (magnitude of thousands). Database servers usually have a limit on the number > of parameters, so reaching this limit causes a failure. > Paging should be introduced to have only a smaller number of parameters in a > single query and do multiple queries to obtain the final result. > > > Diffs > - > > > ambari-server/src/main/java/org/apache/ambari/server/configuration/Configuration.java > 2eb0734 > ambari-server/src/main/java/org/apache/ambari/server/orm/dao/DaoUtils.java > 7f157ec > > ambari-server/src/main/java/org/apache/ambari/server/orm/dao/HostRoleCommandDAO.java > 1768f21 > > Diff: https://reviews.apache.org/r/49265/diff/ > > > Testing > --- > > Running locally... > > > Thanks, > > Daniel Gergely > >
Re: Review Request 49265: When querying host role command entities, the number of parameters should be limited
> On jún. 27, 2016, 5:36 du, Alejandro Fernandez wrote: > > ambari-server/src/main/java/org/apache/ambari/server/orm/dao/HostRoleCommandDAO.java, > > line 288 > > <https://reviews.apache.org/r/49265/diff/1/?file=1431054#file1431054line288> > > > > What's the root cause of more than 1k rows being returned? > > RU/EU? The number of returned lines is irrelevant here. The problem is the number of parameters in the prepared statement. The query above has "task_id IN ?1" part and "?1" is replaced with a collection. If that collection has many elements, the prepared statement looks like "task_id IN (?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,...)". There is an upper limit for the number of parameters on the sql server side. This was handled for oracle only, but mssql has a limit as well, so it has no point to apply the logic only for oracle, rather now it is configurable. The number of parameteres is high when stage summaries are being created and cache is empty (e.g.: because of an earlier host removal or an ambari server restart). That case all tasks are in the collection, so in the case of a 200 nodes cluster the number of tasks can be over 2200. (mssql rejected the request, because the upper limit for number of parameteres in this case was 2100) What does "RU/EU" mean? - Daniel --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/49265/#review139611 ------- On jún. 27, 2016, 2:01 du, Daniel Gergely wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/49265/ > --- > > (Updated jún. 27, 2016, 2:01 du) > > > Review request for Ambari, Laszlo Puskas, Oliver Szabo, Sandor Magyari, Sumit > Mohanty, and Sebastian Toader. > > > Bugs: AMBARI-17449 > https://issues.apache.org/jira/browse/AMBARI-17449 > > > Repository: ambari > > > Description > --- > > In the case of a larger cluster, the number of host role commands can be high > (magnitude of thousands). Database servers usually have a limit on the number > of parameters, so reaching this limit causes a failure. > Paging should be introduced to have only a smaller number of parameters in a > single query and do multiple queries to obtain the final result. > > > Diffs > - > > > ambari-server/src/main/java/org/apache/ambari/server/configuration/Configuration.java > 2eb0734 > ambari-server/src/main/java/org/apache/ambari/server/orm/dao/DaoUtils.java > 7f157ec > > ambari-server/src/main/java/org/apache/ambari/server/orm/dao/HostRoleCommandDAO.java > 1768f21 > > Diff: https://reviews.apache.org/r/49265/diff/ > > > Testing > --- > > Running locally... > > > Thanks, > > Daniel Gergely > >
Review Request 49265: When querying host role command entities, the number of parameters should be limited
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/49265/ --- Review request for Ambari, Laszlo Puskas, Oliver Szabo, Sandor Magyari, Sumit Mohanty, and Sebastian Toader. Bugs: AMBARI-17449 https://issues.apache.org/jira/browse/AMBARI-17449 Repository: ambari Description --- In the case of a larger cluster, the number of host role commands can be high (magnitude of thousands). Database servers usually have a limit on the number of parameters, so reaching this limit causes a failure. Paging should be introduced to have only a smaller number of parameters in a single query and do multiple queries to obtain the final result. Diffs - ambari-server/src/main/java/org/apache/ambari/server/configuration/Configuration.java 2eb0734 ambari-server/src/main/java/org/apache/ambari/server/orm/dao/DaoUtils.java 7f157ec ambari-server/src/main/java/org/apache/ambari/server/orm/dao/HostRoleCommandDAO.java 1768f21 Diff: https://reviews.apache.org/r/49265/diff/ Testing --- Running locally... Thanks, Daniel Gergely
Re: Review Request 49260: Update desired states in case of service restarts
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/49260/#review139563 --- Ship it! Ship It! - Daniel Gergely On jún. 27, 2016, 12:42 du, Laszlo Puskas wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/49260/ > --- > > (Updated jún. 27, 2016, 12:42 du) > > > Review request for Ambari, Daniel Gergely, Oliver Szabo, Robert Nettleton, > and Sandor Magyari. > > > Bugs: AMBARI-17313 > https://issues.apache.org/jira/browse/AMBARI-17313 > > > Repository: ambari > > > Description > --- > > Problem: > In case services / components are restarted, their desired states are not > changed. As during restart -per definition- services are expected to be in > STARTED state this should be OK, however if the restart is triggered for > services in STOPPED state this causes problems (The UI allows triggering > restart for stopped services). (Eg.: recovery manager doesn't bring them back > to running state) > > Solution: > When assembling the RESTART custom command desired states of affected > services are updated to STARTED. > > > Diffs > - > > > ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariCustomCommandExecutionHelper.java > b60592d > > Diff: https://reviews.apache.org/r/49260/diff/ > > > Testing > --- > > Manually tested on dev-env. > > OK > -- > Total run:1073 > Total errors:0 > Total failures:0 > > > Thanks, > > Laszlo Puskas > >
Re: Review Request 49255: Upgrade Solr version to 5.5.2 and use apache archive repo
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/49255/#review139545 --- Ship it! Ship It! - Daniel Gergely On jún. 27, 2016, 9:43 de, Oliver Szabo wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/49255/ > --- > > (Updated jún. 27, 2016, 9:43 de) > > > Review request for Ambari, Daniel Gergely, Miklos Gergely, Sandor Magyari, > and Sumit Mohanty. > > > Bugs: AMBARI-17446 > https://issues.apache.org/jira/browse/AMBARI-17446 > > > Repository: ambari > > > Description > --- > > - upgrade solr version > - use apache archive as default repository (versions not deleted from there) > > > Diffs > - > > ambari-logsearch/ambari-logsearch-assembly/pom.xml c664ffb > ambari-logsearch/pom.xml 2fd1591f > > Diff: https://reviews.apache.org/r/49255/diff/ > > > Testing > --- > > testing done. > > > Thanks, > > Oliver Szabo > >
Re: Review Request 49253: LDAP sync: force to use uid and cn in patterns to check a member is a dn or not
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/49253/#review139544 --- Ship it! ambari-server/src/main/java/org/apache/ambari/server/security/authorization/AmbariLdapBindAuthenticator.java (line 112) <https://reviews.apache.org/r/49253/#comment204812> You can use StringUtils.isEmpty() here. - Daniel Gergely On jún. 27, 2016, 9:25 de, Oliver Szabo wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/49253/ > --- > > (Updated jún. 27, 2016, 9:25 de) > > > Review request for Ambari, Daniel Gergely, Miklos Gergely, Robert Levas, and > Sandor Magyari. > > > Bugs: AMBARI-17444 > https://issues.apache.org/jira/browse/AMBARI-17444 > > > Repository: ambari > > > Description > --- > > - extend check member value is a dn or not (it is possible that > userNameAttribute or groupNameAttribute does not appear in member value) > - make adminGroupMapping part of the BindAuthenticator more readable (+ make > it switchable) and fix if the memberAttribute is not dn > > > Diffs > - > > > ambari-server/src/main/java/org/apache/ambari/server/security/authorization/AmbariLdapBindAuthenticator.java > c63ea92 > > ambari-server/src/main/java/org/apache/ambari/server/security/ldap/AmbariLdapDataPopulator.java > 53ff16d > > ambari-server/src/test/java/org/apache/ambari/server/security/ldap/AmbariLdapDataPopulatorTest.java > 2265eec > > Diff: https://reviews.apache.org/r/49253/diff/ > > > Testing > --- > > All green in apache report > > > Thanks, > > Oliver Szabo > >
Re: Review Request 48266: Add explicit ambari-server log line indicating cluster creation complete
> On jún. 15, 2016, 3:49 du, Laszlo Puskas wrote: > > ambari-server/src/test/java/org/apache/ambari/server/topology/TopologyManagerTest.java, > > line 87 > > <https://reviews.apache.org/r/48266/diff/6/?file=1419580#file1419580line87> > > > > The member annotated with @TestSubject is instantiated by the > > framework. Why do we need two instances here? It is beacuse of the statefullness of TopologyManager. Replay needs a fresh instance of TopologyManager, because the other tests "pollutes" it. - Daniel --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48266/#review137749 ----------- On jún. 16, 2016, 8:24 de, Daniel Gergely wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/48266/ > --- > > (Updated jún. 16, 2016, 8:24 de) > > > Review request for Ambari, Laszlo Puskas, Oliver Szabo, Robert Nettleton, > Sandor Magyari, Sumit Mohanty, and Sebastian Toader. > > > Bugs: AMBARI-17053 > https://issues.apache.org/jira/browse/AMBARI-17053 > > > Repository: ambari > > > Description > --- > > Add a log message to see when the cluster is ready to use (cluster creation > finishes). > > An event after each heartbeat. At that time cluster provision request is > checked if it is finished or not. In case of an ambari server restart, the > replayed requests are used to determine which one was the provision request. > I could not find a nice way to do it, but I could make a workaround. > > > Diffs > - > > > ambari-server/src/main/java/org/apache/ambari/server/actionmanager/ActionDBAccessorImpl.java > 8e6fb3f > > ambari-server/src/main/java/org/apache/ambari/server/events/AmbariEvent.java > 1079806 > > ambari-server/src/main/java/org/apache/ambari/server/events/RequestFinishedEvent.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/topology/PersistedState.java > 77419d8 > > ambari-server/src/main/java/org/apache/ambari/server/topology/PersistedStateImpl.java > 324a397 > > ambari-server/src/main/java/org/apache/ambari/server/topology/TopologyManager.java > e3f5b49 > > ambari-server/src/test/java/org/apache/ambari/server/topology/TopologyManagerTest.java > fd8653c > > Diff: https://reviews.apache.org/r/48266/diff/ > > > Testing > --- > > Succeeded locally > > > Thanks, > > Daniel Gergely > >
Re: Review Request 48266: Add explicit ambari-server log line indicating cluster creation complete
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48266/ --- (Updated jún. 16, 2016, 8:24 de) Review request for Ambari, Laszlo Puskas, Oliver Szabo, Robert Nettleton, Sandor Magyari, Sumit Mohanty, and Sebastian Toader. Bugs: AMBARI-17053 https://issues.apache.org/jira/browse/AMBARI-17053 Repository: ambari Description --- Add a log message to see when the cluster is ready to use (cluster creation finishes). An event after each heartbeat. At that time cluster provision request is checked if it is finished or not. In case of an ambari server restart, the replayed requests are used to determine which one was the provision request. I could not find a nice way to do it, but I could make a workaround. Diffs (updated) - ambari-server/src/main/java/org/apache/ambari/server/actionmanager/ActionDBAccessorImpl.java 8e6fb3f ambari-server/src/main/java/org/apache/ambari/server/events/AmbariEvent.java 1079806 ambari-server/src/main/java/org/apache/ambari/server/events/RequestFinishedEvent.java PRE-CREATION ambari-server/src/main/java/org/apache/ambari/server/topology/PersistedState.java 77419d8 ambari-server/src/main/java/org/apache/ambari/server/topology/PersistedStateImpl.java 324a397 ambari-server/src/main/java/org/apache/ambari/server/topology/TopologyManager.java e3f5b49 ambari-server/src/test/java/org/apache/ambari/server/topology/TopologyManagerTest.java fd8653c Diff: https://reviews.apache.org/r/48266/diff/ Testing --- Succeeded locally Thanks, Daniel Gergely
Re: Review Request 48494: Implement config values trimming for deployment via blueprint
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/48494/#review137731
---
Fix it, then Ship it!
ambari-server/src/main/java/org/apache/ambari/server/controller/internal/DeleteSpacesAtTheEndTrimmingStrategy.java
(line 24)
<https://reviews.apache.org/r/48494/#comment202938>
Please swap variable and constant.
- Daniel Gergely
On jún. 15, 2016, 1:30 du, Dmytro Sen wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48494/
> ---
>
> (Updated jún. 15, 2016, 1:30 du)
>
>
> Review request for Ambari, Robert Nettleton, Sebastian Toader, and Vitalyi
> Brodetskyi.
>
>
> Bugs: AMBARI-17146
> https://issues.apache.org/jira/browse/AMBARI-17146
>
>
> Repository: ambari
>
>
> Description
> ---
>
> Implement config values trimming for deployment via blueprint as we do in UI
>
> trimProperty: function (property) {
> var displayType = Em.get(property, 'displayType');
> var value = Em.get(property, 'value');
> var name = Em.get(property, 'name');
> var rez;
> switch (displayType) {
> case 'directories':
> case 'directory':
> rez = value.replace(/,/g, ' ').trim().split(/\s+/g).join(',');
> break;
> case 'host':
> rez = value.trim();
> break;
> case 'password':
> break;
> default:
> if (name == 'javax.jdo.option.ConnectionURL' || name ==
> 'oozie.service.JPAService.jdbc.url') {
> rez = value.trim();
> }
> rez = (typeof value == 'string') ? value.replace(/(\s+$)/g, '') :
> value;
> }
> return ((rez == '') || (rez == undefined)) ? value : rez;
> },
>
>
> Diffs
> -
>
>
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java
> 9094698
>
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/DefaultTrimmingStrategy.java
> PRE-CREATION
>
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/DeleteSpacesAtTheEndTrimmingStrategy.java
> PRE-CREATION
>
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/DirectoriesTrimmingStrategy.java
> PRE-CREATION
>
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/PasswordTrimmingStrategy.java
> PRE-CREATION
>
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/PropertyValueTrimmingStrategyDefiner.java
> PRE-CREATION
>
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/Stack.java
> ad8d4f9
>
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/TrimmingStrategy.java
> PRE-CREATION
>
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessorTest.java
> cda8fb8
>
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/StackTest.java
> e70af3e
>
> Diff: https://reviews.apache.org/r/48494/diff/
>
>
> Testing
> ---
>
> Unit tests and manual tests passed
>
>
> Thanks,
>
> Dmytro Sen
>
>
Re: Review Request 48494: Implement config values trimming for deployment via blueprint
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/48494/#review137716
---
ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java
(lines 364 - 367)
<https://reviews.apache.org/r/48494/#comment202923>
Since there is no computational expensive calculation in this block, the if
statement can be omitted. isDebugEnabled is used when logging the message needs
some calculation that is used by the debug logging only.
ambari-server/src/main/java/org/apache/ambari/server/controller/internal/DirectoriesTrimmingStrategy.java
(line 25)
<https://reviews.apache.org/r/48494/#comment202927>
Is it possible that an element in the comma separated string contains space?
If it is so, then maybe this whole function could be replaced by:
return stringToTrim.replaceAll("\\s*,+\\s*", ",");
(replacing sequences containing at least one comma and spaces around them
to a single comma. It also removes trailing and leading commas, but preserves
spaces within the elements. You can try it on http://regex101.com)
ambari-server/src/main/java/org/apache/ambari/server/controller/internal/DirectoriesTrimmingStrategy.java
(lines 26 - 33)
<https://reviews.apache.org/r/48494/#comment202926>
If the regex in my previuos comment cannot be used, then you can use
StringUtils to simplify this part:
return StringUtils.join(stringsArray, ",");
ambari-server/src/main/java/org/apache/ambari/server/controller/internal/PropertyValueTrimmingStrategyDefiner.java
(line 41)
<https://reviews.apache.org/r/48494/#comment202928>
Simplify this by swapping the constant and the variable (you can use it as
a thumbrule)
if("directory".equals(type) || "directories".equals(type))
ambari-server/src/main/java/org/apache/ambari/server/controller/internal/PropertyValueTrimmingStrategyDefiner.java
(line 43)
<https://reviews.apache.org/r/48494/#comment202929>
Swap constant and variable
- Daniel Gergely
On jún. 14, 2016, 1:45 du, Dmytro Sen wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48494/
> ---
>
> (Updated jún. 14, 2016, 1:45 du)
>
>
> Review request for Ambari, Robert Nettleton, Sebastian Toader, and Vitalyi
> Brodetskyi.
>
>
> Bugs: AMBARI-17146
> https://issues.apache.org/jira/browse/AMBARI-17146
>
>
> Repository: ambari
>
>
> Description
> ---
>
> Implement config values trimming for deployment via blueprint as we do in UI
>
> trimProperty: function (property) {
> var displayType = Em.get(property, 'displayType');
> var value = Em.get(property, 'value');
> var name = Em.get(property, 'name');
> var rez;
> switch (displayType) {
> case 'directories':
> case 'directory':
> rez = value.replace(/,/g, ' ').trim().split(/\s+/g).join(',');
> break;
> case 'host':
> rez = value.trim();
> break;
> case 'password':
> break;
> default:
> if (name == 'javax.jdo.option.ConnectionURL' || name ==
> 'oozie.service.JPAService.jdbc.url') {
> rez = value.trim();
> }
> rez = (typeof value == 'string') ? value.replace(/(\s+$)/g, '') :
> value;
> }
> return ((rez == '') || (rez == undefined)) ? value : rez;
> },
>
>
> Diffs
> -
>
>
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java
> 9094698
>
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/DefaultTrimmingStrategy.java
> PRE-CREATION
>
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/DeleteSpacesAtTheEndTrimmingStrategy.java
> PRE-CREATION
>
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/DirectoriesTrimmingStrategy.java
> PRE-CREATION
>
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/PasswordTrimmingStrategy.java
> PRE-CREATION
>
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/PropertyValueTrimmingStrategyDefiner.java
> PRE-CREATION
>
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/Stack.java
> ad8d4f9
>
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/TrimmingStrategy.java
> PRE-CREATION
>
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessorTest.java
> cda8fb8
>
> Diff: https://reviews.apache.org/r/48494/diff/
>
>
> Testing
> ---
>
> Unit tests and manual tests passed
>
>
> Thanks,
>
> Dmytro Sen
>
>
Re: Review Request 48642: NPE in ambari-server.out when cluster with kerberos is installed
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48642/ --- (Updated jún. 15, 2016, 11:41 de) Review request for Ambari, Oliver Szabo, Robert Levas, Sandor Magyari, and Sebastian Toader. Changes --- Comment and log message fix Bugs: AMBARI-17196 https://issues.apache.org/jira/browse/AMBARI-17196 Repository: ambari Description --- When creating a cluster with kerberos, ambari-server.out contains NullPointerExceptions, because kerberos related AMBARI_SERVER_ACTIONs do not belong to logical tasks. NPE source is a Long->long conversion when logical task id is null when AMBARI_SERVER_ACTION is tried to be retrieved from logicalTaskMap. The fix is a NPE check that skips the registerPhysicalTaskId method. I added it also to the StartHostsTask class. Diffs (updated) - ambari-server/src/main/java/org/apache/ambari/server/topology/HostRequest.java 00ecb98 Diff: https://reviews.apache.org/r/48642/diff/ Testing --- Changes are done in private classes, so they cannot be accessed from unit tests. Thanks, Daniel Gergely
Re: Review Request 48266: Add explicit ambari-server log line indicating cluster creation complete
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48266/ --- (Updated jún. 15, 2016, 11:33 de) Review request for Ambari, Laszlo Puskas, Oliver Szabo, Robert Nettleton, Sandor Magyari, Sumit Mohanty, and Sebastian Toader. Changes --- Moving event posting from HeartbeatHandler to ActionDBAccessor Bugs: AMBARI-17053 https://issues.apache.org/jira/browse/AMBARI-17053 Repository: ambari Description --- Add a log message to see when the cluster is ready to use (cluster creation finishes). An event after each heartbeat. At that time cluster provision request is checked if it is finished or not. In case of an ambari server restart, the replayed requests are used to determine which one was the provision request. I could not find a nice way to do it, but I could make a workaround. Diffs (updated) - ambari-server/src/main/java/org/apache/ambari/server/actionmanager/ActionDBAccessorImpl.java 8e6fb3f ambari-server/src/main/java/org/apache/ambari/server/events/AmbariEvent.java 1079806 ambari-server/src/main/java/org/apache/ambari/server/events/RequestFinishedEvent.java PRE-CREATION ambari-server/src/main/java/org/apache/ambari/server/topology/PersistedState.java 77419d8 ambari-server/src/main/java/org/apache/ambari/server/topology/PersistedStateImpl.java 324a397 ambari-server/src/main/java/org/apache/ambari/server/topology/TopologyManager.java e3f5b49 ambari-server/src/test/java/org/apache/ambari/server/topology/TopologyManagerTest.java fd8653c Diff: https://reviews.apache.org/r/48266/diff/ Testing --- Succeeded locally Thanks, Daniel Gergely
Re: Review Request 48413: Fix misnamed Zookeeper connect strings in Log Search
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48413/#review137708 --- Ship it! Ship It! - Daniel Gergely On jún. 15, 2016, 9:24 de, Miklos Gergely wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/48413/ > --- > > (Updated jún. 15, 2016, 9:24 de) > > > Review request for Ambari, Daniel Gergely, Oliver Szabo, Robert Nettleton, > Sumit Mohanty, and Sebastian Toader. > > > Bugs: AMBARI-17117 > https://issues.apache.org/jira/browse/AMBARI-17117 > > > Repository: ambari > > > Description > --- > > Variables/properties holding zookeeper connect strings are misnamed as > zk_host, or zk_hosts, which may be misleading. Variable / property names > fixed. > > > Diffs > - > > > ambari-logsearch/ambari-logsearch-logfeeder/src/main/java/org/apache/ambari/logfeeder/logconfig/FetchConfigFromSolr.java > 1f86dd0 > > ambari-logsearch/ambari-logsearch-logfeeder/src/main/java/org/apache/ambari/logfeeder/output/OutputSolr.java > 6fb0b0e > > ambari-logsearch/ambari-logsearch-logfeeder/src/main/java/org/apache/ambari/logfeeder/util/SolrUtil.java > 200a603 > > ambari-logsearch/ambari-logsearch-logfeeder/src/main/resources/config.json.j2 > b6301ca > > ambari-logsearch/ambari-logsearch-logfeeder/src/main/resources/logfeeder.properties > 076c09c > > ambari-logsearch/ambari-logsearch-logfeeder/src/main/resources/output.config.json.j2 > a485600 > > ambari-logsearch/ambari-logsearch-logfeeder/src/test/java/org/apache/ambari/logfeeder/output/OutputSolrTest.java > afbccca > > ambari-logsearch/ambari-logsearch-portal/src/main/java/org/apache/ambari/logsearch/dao/AuditSolrDao.java > 03ff0ff > > ambari-logsearch/ambari-logsearch-portal/src/main/java/org/apache/ambari/logsearch/dao/ServiceLogsSolrDao.java > 14125bc > > ambari-logsearch/ambari-logsearch-portal/src/main/java/org/apache/ambari/logsearch/dao/SolrDaoBase.java > 147e148 > > ambari-logsearch/ambari-logsearch-portal/src/main/java/org/apache/ambari/logsearch/dao/UserConfigSolrDao.java > edf1dcc > > ambari-logsearch/ambari-logsearch-portal/src/main/java/org/apache/ambari/logsearch/solr/metrics/SolrMetricsLoader.java > 21c010f > > ambari-logsearch/ambari-logsearch-portal/src/main/resources/logsearch.properties.j2 > 0a94186 > > ambari-logsearch/ambari-logsearch-solr-client/src/main/java/org/apache/ambari/logsearch/solr/AmbariSolrCloudCLI.java > a2da737 > > ambari-logsearch/ambari-logsearch-solr-client/src/main/java/org/apache/ambari/logsearch/solr/AmbariSolrCloudClient.java > 2805b0b > > ambari-logsearch/ambari-logsearch-solr-client/src/main/java/org/apache/ambari/logsearch/solr/AmbariSolrCloudClientBuilder.java > 0813221 > > ambari-logsearch/ambari-logsearch-solr-client/src/main/java/org/apache/ambari/logsearch/solr/commands/GetSolrHostsCommand.java > f814678 > > ambari-logsearch/ambari-logsearch-solr-client/src/test/java/org/apache/ambari/logsearch/solr/AmbariSolrCloudClientTest.java > c382c14 > > ambari-server/src/main/resources/common-services/LOGSEARCH/0.5.0/package/templates/logfeeder.properties.j2 > 6a52708 > > ambari-server/src/main/resources/common-services/LOGSEARCH/0.5.0/package/templates/logsearch.properties.j2 > 9ada5bf > > ambari-server/src/main/resources/common-services/LOGSEARCH/0.5.0/package/templates/output.config.json.j2 > b31f39b > ambari-server/src/test/python/stacks/2.4/configs/default.json c3aecff > > Diff: https://reviews.apache.org/r/48413/diff/ > > > Testing > --- > > Tested on local cluster > > ambari-logsearch-logfeeder: > Tests run: 35, Failures: 0, Errors: 0, Skipped: 0 > > > Thanks, > > Miklos Gergely > >
Re: Review Request 48642: NPE in ambari-server.out when cluster with kerberos is installed
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48642/ --- (Updated jún. 15, 2016, 8:50 de) Review request for Ambari, Oliver Szabo, Robert Levas, Sandor Magyari, and Sebastian Toader. Changes --- Adding comments and skipping log mesage in case of AMBARI_SERVER_ACTION Bugs: AMBARI-17196 https://issues.apache.org/jira/browse/AMBARI-17196 Repository: ambari Description --- When creating a cluster with kerberos, ambari-server.out contains NullPointerExceptions, because kerberos related AMBARI_SERVER_ACTIONs do not belong to logical tasks. NPE source is a Long->long conversion when logical task id is null when AMBARI_SERVER_ACTION is tried to be retrieved from logicalTaskMap. The fix is a NPE check that skips the registerPhysicalTaskId method. I added it also to the StartHostsTask class. Diffs (updated) - ambari-server/src/main/java/org/apache/ambari/server/topology/HostRequest.java 00ecb98 Diff: https://reviews.apache.org/r/48642/diff/ Testing --- Changes are done in private classes, so they cannot be accessed from unit tests. Thanks, Daniel Gergely
Review Request 48691: Removing and re-adding hosts makes database inconsitent
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48691/ --- Review request for Ambari, Laszlo Puskas, Robert Nettleton, Sandor Magyari, Sumit Mohanty, and Sebastian Toader. Bugs: AMBARI-17219 https://issues.apache.org/jira/browse/AMBARI-17219 Repository: ambari Description --- When a host is removed, it is necessary to have all the components stopped and removed, before the host itself can be deleted. If a host component is not stopped, it is not allowed to be removed, so the host itself cannot be removed. However if re-adding the host is tried, even if it is still in the cluster, a row is inserted to topology_host_info table. Multiple lines of the same host in that table is incorrect, so topology validation fails. Expected behaviour: if host is still in the cluster, a duplicate should not be able to be persisted in any of the related tables. Diffs - ambari-server/src/main/java/org/apache/ambari/server/topology/TopologyManager.java e3f5b49 ambari-server/src/test/java/org/apache/ambari/server/topology/TopologyManagerTest.java fd8653c Diff: https://reviews.apache.org/r/48691/diff/ Testing --- Thanks, Daniel Gergely
Re: Review Request 48266: Add explicit ambari-server log line indicating cluster creation complete
> On jún. 14, 2016, 10:28 de, Sandor Magyari wrote: > > Hi Bob, Summit! > > > > Using events instead of polling continusly seems to be a better solution, > > may be would be better to listen to already existing events like > > ActionFinalReportReceivedEvent which is fired when a command reaches it's > > final state. Probaly even better would be to fire a new event like > > RequestCompleted in ActionDBAccessorImpl.endRequestIfCompleted(). Later we > > may use this event to not just log the end cluster deployment but to > > actually set a final status on LogicalReuqest in db to avoid calculation of > > requestr status all the time. We have to use endRequest method there, because abort calls it directly. Otherwise if everyone thinks it is a better place to post an event, I think I can move the logic there. - Daniel --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48266/#review137477 ----------- On jún. 11, 2016, 6:43 de, Daniel Gergely wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/48266/ > --- > > (Updated jún. 11, 2016, 6:43 de) > > > Review request for Ambari, Laszlo Puskas, Oliver Szabo, Robert Nettleton, > Sandor Magyari, Sumit Mohanty, and Sebastian Toader. > > > Bugs: AMBARI-17053 > https://issues.apache.org/jira/browse/AMBARI-17053 > > > Repository: ambari > > > Description > --- > > Add a log message to see when the cluster is ready to use (cluster creation > finishes). > > An event after each heartbeat. At that time cluster provision request is > checked if it is finished or not. In case of an ambari server restart, the > replayed requests are used to determine which one was the provision request. > I could not find a nice way to do it, but I could make a workaround. > > > Diffs > - > > > ambari-server/src/main/java/org/apache/ambari/server/agent/HeartbeatProcessor.java > c6036c2 > > ambari-server/src/main/java/org/apache/ambari/server/events/AmbariEvent.java > 1079806 > > ambari-server/src/main/java/org/apache/ambari/server/events/HeartbeatProcessingFinishedEvent.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/topology/PersistedState.java > 77419d8 > > ambari-server/src/main/java/org/apache/ambari/server/topology/PersistedStateImpl.java > 324a397 > > ambari-server/src/main/java/org/apache/ambari/server/topology/TopologyManager.java > e3f5b49 > > ambari-server/src/test/java/org/apache/ambari/server/topology/TopologyManagerTest.java > fd8653c > > Diff: https://reviews.apache.org/r/48266/diff/ > > > Testing > --- > > Succeeded locally > > > Thanks, > > Daniel Gergely > >
Re: Review Request 48266: Add explicit ambari-server log line indicating cluster creation complete
> On jún. 13, 2016, 9:23 du, Sumit Mohanty wrote: > > Does this handle UI based cluster deployments? > > Probably there is no good way for UI based deplyments to know if a specific > > request is a cluster create request or not. > > > > What happens after cluster creation is over - does the code continue to > > handle Heartbeat processing completed event? > > Would it have been possible to create an event for Request-Completed and > > process that rather than Heartbeat processing completed? This issue was only about blueprint deployment. UI based deployment is not tracked. If cluster creation is completed, event is still kept sending, however the subscribed method does nothing in this case. So it does not affect performance or does unnecessary calculations. I did not think about tracking request completition, because I felt that the minimum number of checks can be achieved by tracking only heartbeats. - Daniel --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48266/#review137384 ----------- On jún. 11, 2016, 6:43 de, Daniel Gergely wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/48266/ > --- > > (Updated jún. 11, 2016, 6:43 de) > > > Review request for Ambari, Laszlo Puskas, Oliver Szabo, Robert Nettleton, > Sandor Magyari, Sumit Mohanty, and Sebastian Toader. > > > Bugs: AMBARI-17053 > https://issues.apache.org/jira/browse/AMBARI-17053 > > > Repository: ambari > > > Description > --- > > Add a log message to see when the cluster is ready to use (cluster creation > finishes). > > An event after each heartbeat. At that time cluster provision request is > checked if it is finished or not. In case of an ambari server restart, the > replayed requests are used to determine which one was the provision request. > I could not find a nice way to do it, but I could make a workaround. > > > Diffs > - > > > ambari-server/src/main/java/org/apache/ambari/server/agent/HeartbeatProcessor.java > c6036c2 > > ambari-server/src/main/java/org/apache/ambari/server/events/AmbariEvent.java > 1079806 > > ambari-server/src/main/java/org/apache/ambari/server/events/HeartbeatProcessingFinishedEvent.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/topology/PersistedState.java > 77419d8 > > ambari-server/src/main/java/org/apache/ambari/server/topology/PersistedStateImpl.java > 324a397 > > ambari-server/src/main/java/org/apache/ambari/server/topology/TopologyManager.java > e3f5b49 > > ambari-server/src/test/java/org/apache/ambari/server/topology/TopologyManagerTest.java > fd8653c > > Diff: https://reviews.apache.org/r/48266/diff/ > > > Testing > --- > > Succeeded locally > > > Thanks, > > Daniel Gergely > >
Re: Review Request 48266: Add explicit ambari-server log line indicating cluster creation complete
> On jún. 13, 2016, 4:51 du, Robert Nettleton wrote: > > ambari-server/src/main/java/org/apache/ambari/server/agent/HeartbeatProcessor.java, > > line 191 > > <https://reviews.apache.org/r/48266/diff/5/?file=1415529#file1415529line191> > > > > I'm a little concerned that this approach could have performance issues > > down the road. > > > > I'm wondering if it might be worth considering launching a separate > > task thread when the cluster is created by the TopologyManager, that can > > monitor the state of the LogicalRequest as the deployment progresses. > > > > I would recommend asking Sumit to review this as well, since it's not > > clear to me that this is the approach that should be taken. > > Sebastian Toader wrote: > This loop simply iterates through the received reports extracts cluster > id and publishes an event for further processing. The event is consumed on a > separate thread 'ambari-event-bus' which is set up in > ```AmbariEventPublisher```. Do you think that the extraction of cluster name > and cluster id may slow the the heartbeat handler? > > Robert Nettleton wrote: > Hi Sebastian, Thanks for this information. > > I guess I'd be concerned about any extraneous additions to the > HeartBeatHandler, especially given that it seems like there would be other > alternatives. > > Was the possibility of launching a thread in the TopologyManager to > monitor a cluster deployment's status considered? Since the TopologyManager > already has access to the LogicalRequest and the underlying physical requests > as well, it seems like the most natural fit for monitoring status, and > providing a way to log this information. > > My overall concern is that the heartbeat handlers are being modified just > to support logging a message when the cluster has completed. As I mentioned > above, it seems like it would be simpler and more straightforward to resolve > this with a thread that monitors the progress, using the resource APIs that > already exist in Ambari. > > If the approach I've just detailed was considered, can you post why it > was not chosen? Are there some technical concerns that make using a separate > thread not feasible? > > Thanks. Hi Bob, HeartbeatHandler is not modified only to support logging. Now it sends an event (which is basically free: a list is created and cluster ids are stored in that. Cluster ids are from a cache, so there is no performance issue there either), and this event is handled asynchronously (logging part of the code is executed by a different thread). So it does not affect HeartbeatHandler. So it is indeed a separate thread. However if you still have concerns, let's talk, I am open to any other working solution. - Daniel --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48266/#review137326 --- On jún. 11, 2016, 6:43 de, Daniel Gergely wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/48266/ > --- > > (Updated jún. 11, 2016, 6:43 de) > > > Review request for Ambari, Laszlo Puskas, Oliver Szabo, Robert Nettleton, > Sandor Magyari, Sumit Mohanty, and Sebastian Toader. > > > Bugs: AMBARI-17053 > https://issues.apache.org/jira/browse/AMBARI-17053 > > > Repository: ambari > > > Description > --- > > Add a log message to see when the cluster is ready to use (cluster creation > finishes). > > An event after each heartbeat. At that time cluster provision request is > checked if it is finished or not. In case of an ambari server restart, the > replayed requests are used to determine which one was the provision request. > I could not find a nice way to do it, but I could make a workaround. > > > Diffs > - > > > ambari-server/src/main/java/org/apache/ambari/server/agent/HeartbeatProcessor.java > c6036c2 > > ambari-server/src/main/java/org/apache/ambari/server/events/AmbariEvent.java > 1079806 > > ambari-server/src/main/java/org/apache/ambari/server/events/HeartbeatProcessingFinishedEvent.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/topology/PersistedState.java > 77419d8 > > ambari-server/src/main/java/org/apache/ambari/server/topology/PersistedStateImpl.java > 324a397 > > ambari-server/src/main/java/org/apache/ambari/server/topology/TopologyManager.java > e3f5b49 > > ambari-server/src/test/java/org/apache/ambari/server/topology/TopologyManagerTest.java > fd8653c > > Diff: https://reviews.apache.org/r/48266/diff/ > > > Testing > --- > > Succeeded locally > > > Thanks, > > Daniel Gergely > >
Review Request 48642: NPE in ambari-server.out when cluster with kerberos is installed
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48642/ --- Review request for Ambari, Oliver Szabo, Robert Levas, Sandor Magyari, and Sebastian Toader. Bugs: AMBARI-17196 https://issues.apache.org/jira/browse/AMBARI-17196 Repository: ambari Description --- When creating a cluster with kerberos, ambari-server.out contains NullPointerExceptions, because kerberos related AMBARI_SERVER_ACTIONs do not belong to logical tasks. NPE source is a Long->long conversion when logical task id is null when AMBARI_SERVER_ACTION is tried to be retrieved from logicalTaskMap. The fix is a NPE check that skips the registerPhysicalTaskId method. I added it also to the StartHostsTask class. Diffs - ambari-server/src/main/java/org/apache/ambari/server/topology/HostRequest.java 00ecb98 Diff: https://reviews.apache.org/r/48642/diff/ Testing --- Changes are done in private classes, so they cannot be accessed from unit tests. Thanks, Daniel Gergely
Re: Review Request 48494: Implement config values trimming for deployment via blueprint
> On jún. 9, 2016, 4:05 du, Daniel Gergely wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java,
> > line 343
> > <https://reviews.apache.org/r/48494/diff/1/?file=1412628#file1412628line343>
> >
> > Please consider cyclomatic complexity in this method.
>
> Dmytro Sen wrote:
> What is wrong with it ? The loop body is executed only once for every
> property.
> Moreover, this method is executed only once during ambari server
> lifecycle, when the server is least loaded.
Loops have no relations with cyclomatic complexity. I think you wrote about
computational complexity (more specifically time complexity). Cyclomatic
complexity is a measure to show how many possible execution paths are there in
a specific block (eg in a method body or in a class). Each of the **if**
statements increases this number, since there are always 2 options: true or
false.
I counted 11 levels of deepness here, most of them are if statements, which
means a high cyclomatic complexity.
My review was only about avoiding it, because it makes the code less
maintainable and readable. Computational complexity won't be less, business
logic won't be changed, what you gain is maintainibility and in some cases it
can be easier to test.
There are many ways how to reduce it, usually some code can be simplified or a
new method can be introduced. (if you google it up, you will find many
examples. First I would extract methods.)
This is only suggestion, to avoid bad moments for the developer who may need to
change the code in the future.
- Daniel
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/48494/#review136823
---
On jún. 9, 2016, 3:23 du, Dmytro Sen wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48494/
> ---
>
> (Updated jún. 9, 2016, 3:23 du)
>
>
> Review request for Ambari, Robert Nettleton, Sebastian Toader, and Vitalyi
> Brodetskyi.
>
>
> Bugs: AMBARI-17146
> https://issues.apache.org/jira/browse/AMBARI-17146
>
>
> Repository: ambari
>
>
> Description
> ---
>
> Implement config values trimming for deployment via blueprint as we do in UI
>
> trimProperty: function (property) {
> var displayType = Em.get(property, 'displayType');
> var value = Em.get(property, 'value');
> var name = Em.get(property, 'name');
> var rez;
> switch (displayType) {
> case 'directories':
> case 'directory':
> rez = value.replace(/,/g, ' ').trim().split(/\s+/g).join(',');
> break;
> case 'host':
> rez = value.trim();
> break;
> case 'password':
> break;
> default:
> if (name == 'javax.jdo.option.ConnectionURL' || name ==
> 'oozie.service.JPAService.jdbc.url') {
> rez = value.trim();
> }
> rez = (typeof value == 'string') ? value.replace(/(\s+$)/g, '') :
> value;
> }
> return ((rez == '') || (rez == undefined)) ? value : rez;
> },
>
>
> Diffs
> -
>
>
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java
> de70a2c
>
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/Stack.java
> ad8d4f9
>
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessorTest.java
> 9ec0a09
>
> Diff: https://reviews.apache.org/r/48494/diff/
>
>
> Testing
> ---
>
> Unit tests and manual tests passed
>
>
> Thanks,
>
> Dmytro Sen
>
>
Re: Review Request 48494: Implement config values trimming for deployment via blueprint
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/48494/#review136823
---
ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java
(line 343)
<https://reviews.apache.org/r/48494/#comment201909>
Please consider cyclomatic complexity in this method.
- Daniel Gergely
On jún. 9, 2016, 3:23 du, Dmytro Sen wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48494/
> ---
>
> (Updated jún. 9, 2016, 3:23 du)
>
>
> Review request for Ambari, Robert Nettleton, Sebastian Toader, and Vitalyi
> Brodetskyi.
>
>
> Bugs: AMBARI-17146
> https://issues.apache.org/jira/browse/AMBARI-17146
>
>
> Repository: ambari
>
>
> Description
> ---
>
> Implement config values trimming for deployment via blueprint as we do in UI
>
> trimProperty: function (property) {
> var displayType = Em.get(property, 'displayType');
> var value = Em.get(property, 'value');
> var name = Em.get(property, 'name');
> var rez;
> switch (displayType) {
> case 'directories':
> case 'directory':
> rez = value.replace(/,/g, ' ').trim().split(/\s+/g).join(',');
> break;
> case 'host':
> rez = value.trim();
> break;
> case 'password':
> break;
> default:
> if (name == 'javax.jdo.option.ConnectionURL' || name ==
> 'oozie.service.JPAService.jdbc.url') {
> rez = value.trim();
> }
> rez = (typeof value == 'string') ? value.replace(/(\s+$)/g, '') :
> value;
> }
> return ((rez == '') || (rez == undefined)) ? value : rez;
> },
>
>
> Diffs
> -
>
>
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java
> de70a2c
>
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/Stack.java
> ad8d4f9
>
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessorTest.java
> 9ec0a09
>
> Diff: https://reviews.apache.org/r/48494/diff/
>
>
> Testing
> ---
>
> Unit tests and manual tests passed
>
>
> Thanks,
>
> Dmytro Sen
>
>
Re: Review Request 48266: Add explicit ambari-server log line indicating cluster creation complete
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48266/ --- (Updated jún. 9, 2016, 12:54 du) Review request for Ambari, Laszlo Puskas, Oliver Szabo, Robert Nettleton, Sandor Magyari, Sumit Mohanty, and Sebastian Toader. Changes --- Review fixes Bugs: AMBARI-17053 https://issues.apache.org/jira/browse/AMBARI-17053 Repository: ambari Description --- Add a log message to see when the cluster is ready to use (cluster creation finishes). An event after each heartbeat. At that time cluster provision request is checked if it is finished or not. In case of an ambari server restart, the replayed requests are used to determine which one was the provision request. I could not find a nice way to do it, but I could make a workaround. Diffs (updated) - ambari-server/src/main/java/org/apache/ambari/server/agent/HeartbeatProcessor.java c6036c2 ambari-server/src/main/java/org/apache/ambari/server/events/AmbariEvent.java 1079806 ambari-server/src/main/java/org/apache/ambari/server/events/HeartbeatProcessingFinishedEvent.java PRE-CREATION ambari-server/src/main/java/org/apache/ambari/server/topology/PersistedState.java 77419d8 ambari-server/src/main/java/org/apache/ambari/server/topology/PersistedStateImpl.java 324a397 ambari-server/src/main/java/org/apache/ambari/server/topology/TopologyManager.java e3f5b49 ambari-server/src/test/java/org/apache/ambari/server/topology/TopologyManagerTest.java fd8653c Diff: https://reviews.apache.org/r/48266/diff/ Testing --- Still running locally... Thanks, Daniel Gergely
Re: Review Request 48266: Add explicit ambari-server log line indicating cluster creation complete
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48266/ --- (Updated jún. 9, 2016, 12:12 du) Review request for Ambari, Laszlo Puskas, Oliver Szabo, Robert Nettleton, Sandor Magyari, Sumit Mohanty, and Sebastian Toader. Changes --- Handling multiple clusters, acquire provision request in an alternative way by extending PersistedState interface. Bugs: AMBARI-17053 https://issues.apache.org/jira/browse/AMBARI-17053 Repository: ambari Description --- Add a log message to see when the cluster is ready to use (cluster creation finishes). An event after each heartbeat. At that time cluster provision request is checked if it is finished or not. In case of an ambari server restart, the replayed requests are used to determine which one was the provision request. I could not find a nice way to do it, but I could make a workaround. Diffs (updated) - ambari-server/src/main/java/org/apache/ambari/server/agent/HeartbeatProcessor.java c6036c2 ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ProvisionClusterRequest.java 3feac55 ambari-server/src/main/java/org/apache/ambari/server/events/AmbariEvent.java 1079806 ambari-server/src/main/java/org/apache/ambari/server/events/HeartbeatProcessingFinishedEvent.java PRE-CREATION ambari-server/src/main/java/org/apache/ambari/server/topology/PersistedState.java 77419d8 ambari-server/src/main/java/org/apache/ambari/server/topology/PersistedStateImpl.java 324a397 ambari-server/src/main/java/org/apache/ambari/server/topology/TopologyManager.java e3f5b49 ambari-server/src/test/java/org/apache/ambari/server/topology/TopologyManagerTest.java fd8653c Diff: https://reviews.apache.org/r/48266/diff/ Testing --- Still running locally... Thanks, Daniel Gergely
Re: Review Request 48266: Add explicit ambari-server log line indicating cluster creation complete
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48266/ --- (Updated jún. 7, 2016, 7:47 de) Review request for Ambari, Laszlo Puskas, Oliver Szabo, Robert Nettleton, Sandor Magyari, Sumit Mohanty, and Sebastian Toader. Bugs: AMBARI-17053 https://issues.apache.org/jira/browse/AMBARI-17053 Repository: ambari Description --- Add a log message to see when the cluster is ready to use (cluster creation finishes). An event after each heartbeat. At that time cluster provision request is checked if it is finished or not. In case of an ambari server restart, the replayed requests are used to determine which one was the provision request. I could not find a nice way to do it, but I could make a workaround. Diffs (updated) - ambari-server/src/main/java/org/apache/ambari/server/agent/HeartbeatProcessor.java c6036c2 ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ProvisionClusterRequest.java 3feac55 ambari-server/src/main/java/org/apache/ambari/server/events/AmbariEvent.java 1079806 ambari-server/src/main/java/org/apache/ambari/server/topology/TopologyManager.java e3f5b49 ambari-server/src/test/java/org/apache/ambari/server/topology/TopologyManagerTest.java fd8653c Diff: https://reviews.apache.org/r/48266/diff/ Testing --- Still running locally... Thanks, Daniel Gergely
Review Request 48266: Add explicit ambari-server log line indicating cluster creation complete
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48266/ --- Review request for Ambari, Laszlo Puskas, Oliver Szabo, Robert Nettleton, Sandor Magyari, Sumit Mohanty, and Sebastian Toader. Bugs: AMBARI-17053 https://issues.apache.org/jira/browse/AMBARI-17053 Repository: ambari Description --- Add a log message to see when the cluster is ready to use (cluster creation finishes). An event after each heartbeat. At that time cluster provision request is checked if it is finished or not. In case of an ambari server restart, the replayed requests are used to determine which one was the provision request. I could not find a nice way to do it, but I could make a workaround. Diffs - ambari-server/src/main/java/org/apache/ambari/server/agent/HeartbeatProcessor.java c6036c2 ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ProvisionClusterRequest.java 3feac55 ambari-server/src/main/java/org/apache/ambari/server/events/AmbariEvent.java 1079806 ambari-server/src/main/java/org/apache/ambari/server/topology/TopologyManager.java e3f5b49 ambari-server/src/test/java/org/apache/ambari/server/topology/TopologyManagerTest.java fd8653c Diff: https://reviews.apache.org/r/48266/diff/ Testing --- Still running locally... Thanks, Daniel Gergely
Re: Review Request 47976: LDAP sync cannot handle if the member attribute value is not DN or id
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47976/#review135472 --- Ship it! Ship It! - Daniel Gergely On máj. 27, 2016, 8:14 du, Oliver Szabo wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/47976/ > --- > > (Updated máj. 27, 2016, 8:14 du) > > > Review request for Ambari, Daniel Gergely, Robert Levas, Robert Nettleton, > and Sebastian Toader. > > > Bugs: AMBARI-16875 > https://issues.apache.org/jira/browse/AMBARI-16875 > > > Repository: ambari > > > Description > --- > > In some rare cases, member attribute value for a group/user can be > constructed. (not baseDN/uid, sometimes ldap proxies does that) > > Added 2 feature to fix these problems (to manipulate queries that are used > during sync): > > 2.1.) use regexp to get the useful informations from a custom member > attribute value: (for groups/users) > "authentication.ldap.sync.userMemberReplacePattern" > "authentication.ldap.sync.groupMemberReplacePattern" > > e.g.: > member:
Re: Review Request 47949: AMBARI-16923. Fix for getting the 'hive.llap.daemon.queue.name' config Property Attributes updated if there is a change in 'capacity-scheduler'.
> On máj. 27, 2016, 12:09 du, Daniel Gergely wrote: > > Just a quiestion: if \n separated string is needed only for the UI, wouldnt it be better to handle it on the UI side? So UI can do the conversion, since it is not needed anywhere else. If it is also used somewhere else, then fine. - Daniel --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47949/#review135224 --- On máj. 27, 2016, 10:48 de, Swapan Shridhar wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/47949/ > --- > > (Updated máj. 27, 2016, 10:48 de) > > > Review request for Ambari, Alejandro Fernandez, Daniel Gergely, and Sumit > Mohanty. > > > Bugs: AMBARI-16923 > https://issues.apache.org/jira/browse/AMBARI-16923 > > > Repository: ambari > > > Description > --- > > - We need to read 'capacity-schdeuler' : >1. Firstly, from configurations where the configs can either be a > dictionary or single "\n" string. It will be there if there is a change done > to it as part of current invocation. If they are not there, then go ahead and > read: >2. services for the configs where again it can either be a a dictionary or > single "\n" string. > > > - Removed stack_advisor.py 221-234 lines as they are no more valid as we dont > do calulation based on change to cache value now. > > > Diffs > - > > ambari-server/src/main/resources/stacks/HDP/2.5/services/stack_advisor.py > 8c5351f > ambari-server/src/test/python/stacks/2.5/common/test_stack_advisor.py > 0066e1d > > Diff: https://reviews.apache.org/r/47949/diff/ > > > Testing > --- > > - Python UT added. > - > - Python UT passes. > > > Thanks, > > Swapan Shridhar > >
Re: Review Request 47946: Yarn minimum container size calculation problem in stack advisor
> On máj. 27, 2016, 11:05 de, Swapan Shridhar wrote: > > ambari-server/src/main/resources/stacks/HDP/2.5/services/stack_advisor.py, > > line 643 > > <https://reviews.apache.org/r/47946/diff/1/?file=1396190#file1396190line643> > > > > Should we do max here, or just pick configurations if > > 'yarn.scheduler.minimum-allocation-mb' value is there, as that's the latest > > value in terms of timeline compared to services. Your inputs please? Basically I asked the same question. Services array might not needed at all, since the output of the property calculation (for yarn) is in the configurations array, so we can use it and that is the most accurate one. But services is used at many other places, so I did not want to remove it completely, because I might broke something. - Daniel --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47946/#review135215 ----------- On máj. 27, 2016, 10:14 de, Daniel Gergely wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/47946/ > --- > > (Updated máj. 27, 2016, 10:14 de) > > > Review request for Ambari, Balázs Bence Sári, Laszlo Puskas, Oliver Szabo, > Sandor Magyari, Sumit Mohanty, Swapan Shridhar, and Sebastian Toader. > > > Bugs: AMBARI-16921 > https://issues.apache.org/jira/browse/AMBARI-16921 > > > Repository: ambari > > > Description > --- > > Stack advisor gets yarn minimum container size property from only the > "services" input, but does not take into account the already calculated > configurations. > As a result memory size for hive interactive server is not correct, so it > doesn't start. > > Now services and configurations array are both considered. Does services even > needed here? > > > Diffs > - > > ambari-server/src/main/resources/stacks/HDP/2.5/services/stack_advisor.py > 8c5351f > > Diff: https://reviews.apache.org/r/47946/diff/ > > > Testing > --- > > [INFO] > > [INFO] BUILD SUCCESS > [INFO] > > [INFO] Total time: 2:45.484s > [INFO] Finished at: Fri May 27 12:05:56 CEST 2016 > [INFO] Final Memory: 119M/797M > [INFO] > > > > Thanks, > > Daniel Gergely > >
Re: Review Request 47946: Yarn minimum container size calculation problem in stack advisor
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47946/ --- (Updated máj. 27, 2016, 10:14 de) Review request for Ambari, Balázs Bence Sári, Laszlo Puskas, Oliver Szabo, Sandor Magyari, Sumit Mohanty, Swapan Shridhar, and Sebastian Toader. Bugs: AMBARI-16921 https://issues.apache.org/jira/browse/AMBARI-16921 Repository: ambari Description (updated) --- Stack advisor gets yarn minimum container size property from only the "services" input, but does not take into account the already calculated configurations. As a result memory size for hive interactive server is not correct, so it doesn't start. Now services and configurations array are both considered. Does services even needed here? Diffs - ambari-server/src/main/resources/stacks/HDP/2.5/services/stack_advisor.py 8c5351f Diff: https://reviews.apache.org/r/47946/diff/ Testing --- [INFO] [INFO] BUILD SUCCESS [INFO] [INFO] Total time: 2:45.484s [INFO] Finished at: Fri May 27 12:05:56 CEST 2016 [INFO] Final Memory: 119M/797M [INFO] Thanks, Daniel Gergely
Review Request 47946: Yarn minimum container size calculation problem in stack advisor
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47946/ --- Review request for Ambari, Balázs Bence Sári, Laszlo Puskas, Oliver Szabo, Sandor Magyari, Sumit Mohanty, Swapan Shridhar, and Sebastian Toader. Bugs: AMBARI-16921 https://issues.apache.org/jira/browse/AMBARI-16921 Repository: ambari Description --- Stack advisor gets yarn minimum container size property from only the "services" input, but does not take into account the already calculated configurations. As a result memory size for hive interactive server is not correct, so it doesn't start. Diffs - ambari-server/src/main/resources/stacks/HDP/2.5/services/stack_advisor.py 8c5351f Diff: https://reviews.apache.org/r/47946/diff/ Testing --- [INFO] [INFO] BUILD SUCCESS [INFO] [INFO] Total time: 2:45.484s [INFO] Finished at: Fri May 27 12:05:56 CEST 2016 [INFO] Final Memory: 119M/797M [INFO] Thanks, Daniel Gergely
Re: Review Request 47596: HiveServer interactive - incorrect default memory value
> On máj. 24, 2016, 4:03 de, Swapan Shridhar wrote: > > ambari-server/src/main/resources/stacks/HDP/2.5/services/stack_advisor.py, > > line 221 > > <https://reviews.apache.org/r/47596/diff/2/?file=1388878#file1388878line221> > > > > Can you explain how moving this line up helps ? Thx. I moved it up to make sure that this setting has a correct value all the time, even if calculations are skipped. Skip points have been added, so tracking them and add this value everywhere would not be a good solution. Moving the line up makes sure that the initial value for that property is correct, but later in the function it can be overwritten with a calculated value (if those parts are not skipped). If the memory setting is not correct, then hive interactive server won't start. - Daniel --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47596/#review134511 ------- On máj. 23, 2016, 8:20 de, Daniel Gergely wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/47596/ > --- > > (Updated máj. 23, 2016, 8:20 de) > > > Review request for Ambari, Laszlo Puskas, Oliver Szabo, Sandor Magyari, Sumit > Mohanty, and Sebastian Toader. > > > Bugs: AMBARI-16776 > https://issues.apache.org/jira/browse/AMBARI-16776 > > > Repository: ambari > > > Description > --- > > Default value of hive.llap.daemon.yarn.container.mb is not aligned to > yarn.scheduler.minimum-allocation-mb > If the former one is less than the latter one, then hive server interactive > wont start. So the default values must be in a proper relation. > > > Diffs > - > > > ambari-server/src/main/resources/common-services/HIVE/0.12.0.2.0/package/scripts/params_linux.py > 9f5d799 > > ambari-server/src/main/resources/stacks/HDP/2.5/services/HIVE/configuration/hive-interactive-site.xml > db5f616 > ambari-server/src/main/resources/stacks/HDP/2.5/services/stack_advisor.py > 11aac72 > > Diff: https://reviews.apache.org/r/47596/diff/ > > > Testing > --- > > [INFO] > > [INFO] BUILD SUCCESS > [INFO] > > [INFO] Total time: 2:15.746s > [INFO] Finished at: Thu May 19 18:52:43 CEST 2016 > [INFO] Final Memory: 58M/1120M > [INFO] > > > > Thanks, > > Daniel Gergely > >
Re: Review Request 47726: Blueprint Export does not replace hive.llap.zk.sm.connectionString
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47726/ --- (Updated máj. 24, 2016, 8:55 de) Review request for Ambari, Balázs Bence Sári, Laszlo Puskas, Oliver Szabo, Robert Nettleton, Sandor Magyari, Sumit Mohanty, and Sebastian Toader. Changes --- Unit test added Bugs: AMBARI-16820 https://issues.apache.org/jira/browse/AMBARI-16820 Repository: ambari Description --- MultiHostUpdater was not assigned to the property hive.llap.zk.sm.connectionString Diffs (updated) - ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java 9cabdbb ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessorTest.java 073e827 Diff: https://reviews.apache.org/r/47726/diff/ Testing --- In progress... Thanks, Daniel Gergely
Re: Review Request 47726: Blueprint Export does not replace hive.llap.zk.sm.connectionString
> On máj. 23, 2016, 2:59 du, Robert Nettleton wrote: > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java, > > line 2395 > > <https://reviews.apache.org/r/47726/diff/1/?file=1391619#file1391619line2395> > > > > +1 for Sebastian's comment, a unit test should definitely be added here > > to verify this change. > > Daniel Gergely wrote: > We were talking about this unit test thing. These are static variables, > so I can test if all the properties are there in the map. However it is not > easy to test whether the correct PropertyUpdater is used with the correct > component. These are not visible from outside, so a complex mocking or using > Reflection could made it possible, but these are messy things. So he dropped > the issue here. > > Robert Nettleton wrote: > Hi Daniel, > > It should be pretty simple to unit test this issue, by verifiying that > this property is updated as expected, when the expected set of components are > included in the Blueprint/Cluster Configuration. There is no need to verify > the static maps being modified, but there is definitely a need to make sure > that this property is handled correctly by the Blueprint export process, so I > still think a unit test is necessary here. > > Thanks. > > Robert Nettleton wrote: > Here's an example of a unit test that verifies that the property updaters > are handling Blueprint exports correctly: > > > org.apache.ambari.server.controller.internal.BlueprintConfigurationProcessorTest#testYarnConfigExported > > There are many more examples of this in this test case as well. > > Thanks. I added the test. However I have a different opinion on how to build up tests. Blueprint processor is way too complex, so it is hard to separate responsibilities. In an ideal world I would write tests for the property updaters, and not for all the properties that use these updaters. If property updaters are executed successfully, then all the properties that use them must be replaced correctly. Testing if all the properties have the correct updater with correct parameter is a separated responsibility. In the case we want to test whether replacement for each property is done correctly, that is rather a system test than a unit test, since we need a complete system to test on. No offense, this is just my way of thinking. - Daniel --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47726/#review134358 --- On máj. 23, 2016, 2:07 du, Daniel Gergely wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/47726/ > --- > > (Updated máj. 23, 2016, 2:07 du) > > > Review request for Ambari, Balázs Bence Sári, Laszlo Puskas, Oliver Szabo, > Robert Nettleton, Sandor Magyari, Sumit Mohanty, and Sebastian Toader. > > > Bugs: AMBARI-16820 > https://issues.apache.org/jira/browse/AMBARI-16820 > > > Repository: ambari > > > Description > --- > > MultiHostUpdater was not assigned to the property > hive.llap.zk.sm.connectionString > > > Diffs > - > > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java > 9cabdbb > > Diff: https://reviews.apache.org/r/47726/diff/ > > > Testing > --- > > In progress... > > > Thanks, > > Daniel Gergely > >
Review Request 47726: Blueprint Export does not replace hive.llap.zk.sm.connectionString
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47726/ --- Review request for Ambari, Balázs Bence Sári, Laszlo Puskas, Oliver Szabo, Sandor Magyari, Sumit Mohanty, and Sebastian Toader. Bugs: AMBARI-16820 https://issues.apache.org/jira/browse/AMBARI-16820 Repository: ambari Description --- MultiHostUpdater was not assigned to the property hive.llap.zk.sm.connectionString Diffs - ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java 9cabdbb Diff: https://reviews.apache.org/r/47726/diff/ Testing --- In progress... Thanks, Daniel Gergely
Review Request 47596: HiveServer interactive - incorrect default memory value
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47596/ --- Review request for Ambari, Laszlo Puskas, Oliver Szabo, Sandor Magyari, and Sebastian Toader. Bugs: AMBARI-16776 https://issues.apache.org/jira/browse/AMBARI-16776 Repository: ambari Description --- Default value of hive.llap.daemon.yarn.container.mb is not aligned to yarn.scheduler.minimum-allocation-mb If the former one is less than the latter one, then hive server interactive wont start. So the default values must be in a proper relation. Diffs - ambari-server/src/main/resources/stacks/HDP/2.5/services/HIVE/configuration/hive-interactive-site.xml db5f616 Diff: https://reviews.apache.org/r/47596/diff/ Testing --- [INFO] [INFO] BUILD SUCCESS [INFO] [INFO] Total time: 2:15.746s [INFO] Finished at: Thu May 19 18:52:43 CEST 2016 [INFO] Final Memory: 58M/1120M [INFO] Thanks, Daniel Gergely
Re: Review Request 47297: Extend logging for ActionQueue's retry logic
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47297/ --- (Updated máj. 17, 2016, 7:31 de) Review request for Ambari, Balázs Bence Sári, Laszlo Puskas, Oliver Szabo, Sandor Magyari, Sumit Mohanty, and Sebastian Toader. Changes --- TaskId is added to the log message Bugs: AMBARI-16630 https://issues.apache.org/jira/browse/AMBARI-16630 Repository: ambari Description --- At info level, commands should be tracked if they are retried on failure or not, and if not, what is the reason for that. Diffs (updated) - ambari-agent/src/main/python/ambari_agent/ActionQueue.py 85389f9 ambari-agent/src/test/python/ambari_agent/TestActionQueue.py 7d0efa2 Diff: https://reviews.apache.org/r/47297/diff/ Testing --- [INFO] [INFO] BUILD SUCCESS [INFO] [INFO] Total time: 1:19.911s [INFO] Finished at: Thu May 12 10:58:43 CEST 2016 [INFO] Final Memory: 17M/850M [INFO] Thanks, Daniel Gergely
Re: Review Request 47117: HiveServer interactive fails to start
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47117/ --- (Updated máj. 11, 2016, 2:53 du) Review request for Ambari, Balázs Bence Sári, Laszlo Puskas, Oliver Szabo, Sandor Magyari, and Sebastian Toader. Changes --- Memory setting is needed to be added even if other llap related configs are skipped. (hive.llap.daemon.yarn.container.mb must be greater or equal to yarn.scheduler.allocation-minimum-mb, otherwise hive server interactive does not start) Bugs: AMBARI-16303 https://issues.apache.org/jira/browse/AMBARI-16303 Repository: ambari Description --- If Total Daemon Size of hive server interactive is less than yarn container minimum size, then hive server interactive fails to start. Stack advisor was corrected to take yarn container size into account. The property *hive_server_interactive_host* was not resolved, not it is. It was simply missing from the BlueprintConfigurationProcessor. Diffs (updated) - ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java 29f937a ambari-server/src/main/resources/stacks/HDP/2.5/services/stack_advisor.py 37e6ef6 ambari-server/src/test/python/stacks/2.5/common/test_stack_advisor.py e9ddc25 Diff: https://reviews.apache.org/r/47117/diff/ Testing --- The stack advisor issue is checked manually. The blueprint issue is also checked manually, a separated unit test checks if the resolver itself works correctly. Tests run: 4312, Failures: 0, Errors: 7, Skipped: 34 (failing tests are not related to this issue) Thanks, Daniel Gergely
Re: Review Request 47117: HiveServer interactive fails to start
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47117/ --- (Updated máj. 9, 2016, 2:18 du) Review request for Ambari, Balázs Bence Sári, Laszlo Puskas, Oliver Szabo, Sandor Magyari, and Sebastian Toader. Changes --- Unit test report Bugs: AMBARI-16303 https://issues.apache.org/jira/browse/AMBARI-16303 Repository: ambari Description --- If Total Daemon Size of hive server interactive is less than yarn container minimum size, then hive server interactive fails to start. Stack advisor was corrected to take yarn container size into account. The property *hive_server_interactive_host* was not resolved, not it is. It was simply missing from the BlueprintConfigurationProcessor. Diffs - ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java 29f937a ambari-server/src/main/resources/stacks/HDP/2.5/services/stack_advisor.py 37e6ef6 Diff: https://reviews.apache.org/r/47117/diff/ Testing (updated) --- The stack advisor issue is checked manually. The blueprint issue is also checked manually, a separated unit test checks if the resolver itself works correctly. Tests run: 4312, Failures: 0, Errors: 7, Skipped: 34 (failing tests are not related to this issue) Thanks, Daniel Gergely
Re: Review Request 47117: HiveServer interactive fails to start
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47117/ --- (Updated máj. 9, 2016, 1:01 du) Review request for Ambari, Balázs Bence Sári, Laszlo Puskas, Oliver Szabo, Sandor Magyari, and Sebastian Toader. Bugs: AMBARI-16303 https://issues.apache.org/jira/browse/AMBARI-16303 Repository: ambari Description --- If Total Daemon Size of hive server interactive is less than yarn container minimum size, then hive server interactive fails to start. Stack advisor was corrected to take yarn container size into account. The property *hive_server_interactive_host* was not resolved, not it is. It was simply missing from the BlueprintConfigurationProcessor. Diffs (updated) - ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java 29f937a ambari-server/src/main/resources/stacks/HDP/2.5/services/stack_advisor.py 37e6ef6 Diff: https://reviews.apache.org/r/47117/diff/ Testing --- The stack advisor issue is checked manually. The blueprint issue is also checked manually, a separated unit test checks if the resolver itself works correctly. Unit tests are still running locally... Thanks, Daniel Gergely
Re: Review Request 47014: Blueprint processor should create ConfigGroup even with only one host registered
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47014/#review131828 --- Fix it, then Ship it! ambari-server/src/test/java/org/apache/ambari/server/topology/AmbariContextTest.java (line 394) <https://reviews.apache.org/r/47014/#comment195870> Use 3 separated Asserts to help debugging in case on an error. - Daniel Gergely On máj. 5, 2016, 2:12 du, Sebastian Toader wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/47014/ > --- > > (Updated máj. 5, 2016, 2:12 du) > > > Review request for Ambari, Daniel Gergely, Laszlo Puskas, Sandor Magyari, > Srimanth Gunturi, and Sid Wagle. > > > Bugs: AMBARI-16270 > https://issues.apache.org/jira/browse/AMBARI-16270 > > > Repository: ambari > > > Description > --- > > When the first host for a host group registers with the cluster and is > assigned to host group Ambari checks if there is any config group to be > created for that host group. If it has to than create a config group and > assigning to it all hosts that are expected for the parent host group. > However it not checks is all of the expected hosts of the host group have > already been associated with the cluster thus the config group creation fails > as config groups can have only hosts added that already are associated with > the cluster. > > This has been changed to add only those hosts to config group that already > have been associated with the cluster and update the created config group > with the remaing hosts as these become available and register with the > cluster. > > > Diffs > - > > > ambari-server/src/main/java/org/apache/ambari/server/topology/AmbariContext.java > cf1a6ac > > ambari-server/src/test/java/org/apache/ambari/server/topology/AmbariContextTest.java > 1613d11 > > Diff: https://reviews.apache.org/r/47014/diff/ > > > Testing > --- > > Manual testing done. > > Unit tests are in progress. > > > Thanks, > > Sebastian Toader > >
Review Request 46889: Improve output of command execution retry logic on agents
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46889/ --- Review request for Ambari, Laszlo Puskas, Oliver Szabo, Sandor Magyari, Sumit Mohanty, and Sebastian Toader. Bugs: AMBARI-16199 https://issues.apache.org/jira/browse/AMBARI-16199 Repository: ambari Description --- When command execution is retried on agent, the output of the command can be misleading if the first try failed, but the second was successful. In that case error message remains in the output to indicate that there was a recovered problem. Additional information was added here to indicate that a retry happened and a summary line that states whether eventually the command was successful or not. Diffs - ambari-agent/src/main/python/ambari_agent/ActionQueue.py c5340a0 ambari-agent/src/test/python/ambari_agent/TestActionQueue.py bca506e Diff: https://reviews.apache.org/r/46889/diff/ Testing --- I did manual testing to see the output messages on the UI. Thanks, Daniel Gergely
Re: Review Request 46695: User imported from AD is unable to login to Ambari
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46695/#review130615 --- Ship it! Ship It! - Daniel Gergely On ápr. 26, 2016, 2:17 du, Sebastian Toader wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/46695/ > --- > > (Updated ápr. 26, 2016, 2:17 du) > > > Review request for Ambari, Daniel Gergely, Oliver Szabo, and Sandor Magyari. > > > Bugs: AMBARI-16119 > https://issues.apache.org/jira/browse/AMBARI-16119 > > > Repository: ambari > > > Description > --- > > When user authenticates againts AD the user details are pulled (ldap binding) > from AD. In case the user logged in with a login alias (e.g. when a user is > present in multiple subdomains within a forest than the user name appears in > multiple places. In this case the user has to login with a login alias that > contains domain information which uniquelly identifies the user in AD) Ambari > created an override for the user detail behind the scenes in order to replace > the login user name with the ambari user name that maps to it. > > The override is nothing else than copying all fields from origin user details > object but user name. Among the fields being copied over there is user > password which apparently is populated when OpenLDAP is used however in case > of AD its left null. The override user details object Ambari creates always > expects a non-null password thus the creation of it failed when AD was used. > > > The overriding of user details has been modified to pass empty string as > password is the passowrd in the original user details object is null. > > Also some optimisation was added to create the override if the user logged in > with a login alias. > > > Diffs > - > > > ambari-server/src/main/java/org/apache/ambari/server/security/authorization/AmbariAuthentication.java > 98b97b2 > > Diff: https://reviews.apache.org/r/46695/diff/ > > > Testing > --- > > Tested manually on both OpenLDAP and AD. > > Unit tests are in progress. > > > Thanks, > > Sebastian Toader > >
Re: Review Request 46695: User imported from AD is unable to login to Ambari
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46695/#review130611 --- ambari-server/src/main/java/org/apache/ambari/server/security/authorization/AmbariAuthentication.java (line 195) <https://reviews.apache.org/r/46695/#comment194429> Why is password assigned to the usernameOrig? - Daniel Gergely On ápr. 26, 2016, 1:55 du, Sebastian Toader wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/46695/ > --- > > (Updated ápr. 26, 2016, 1:55 du) > > > Review request for Ambari, Daniel Gergely, Oliver Szabo, and Sandor Magyari. > > > Bugs: AMBARI-16119 > https://issues.apache.org/jira/browse/AMBARI-16119 > > > Repository: ambari > > > Description > --- > > When user authenticates againts AD the user details are pulled (ldap binding) > from AD. In case the user logged in with a login alias (e.g. when a user is > present in multiple subdomains within a forest than the user name appears in > multiple places. In this case the user has to login with a login alias that > contains domain information which uniquelly identifies the user in AD) Ambari > created an override for the user detail behind the scenes in order to replace > the login user name with the ambari user name that maps to it. > > The override is nothing else than copying all fields from origin user details > object but user name. Among the fields being copied over there is user > password which apparently is populated when OpenLDAP is used however in case > of AD its left null. The override user details object Ambari creates always > expects a non-null password thus the creation of it failed when AD was used. > > > The overriding of user details has been modified to pass empty string as > password is the passowrd in the original user details object is null. > > Also some optimisation was added to create the override if the user logged in > with a login alias. > > > Diffs > - > > > ambari-server/src/main/java/org/apache/ambari/server/security/authorization/AmbariAuthentication.java > 98b97b2 > > Diff: https://reviews.apache.org/r/46695/diff/ > > > Testing > --- > > Tested manually on both OpenLDAP and AD. > > Unit tests are in progress. > > > Thanks, > > Sebastian Toader > >
Re: Review Request 46434: DataNode and RegionServer during upgrade are reported as "failed" incorrectly
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46434/ --- (Updated ápr. 22, 2016, 12:42 du) Review request for Ambari, Alejandro Fernandez, Miklos Gergely, Oliver Szabo, Sandor Magyari, and Sebastian Toader. Changes --- Review fixes Bugs: AMBARI-15991 https://issues.apache.org/jira/browse/AMBARI-15991 Repository: ambari Description --- If upgrade process takes longer than expected, DataNode and RegionServer is reported as failed. It happens because it needs more time to finish update. The fix for RegionServer checks if the process is running and if it is so, then it is not considered as a failure. For DataNode the process is also checked and if it is running then check is repeated 2 times with 5 minutes wait. I had a limitation here, python scripts are allowed to run for 20 minutes by default and this checking takes 16 mins (2 minutes initial check, 5 minutes sleep if there is a failure, 2 minutes regaular check, 5 minutes sleep, 2 minutes final check). If more time is needed, then default value of *server.task.timeout* and number of repetition in 5 minutes check should be increased. Diffs (updated) - ambari-server/src/main/resources/common-services/HBASE/0.96.0.2.0/package/scripts/upgrade.py 01a8156 ambari-server/src/main/resources/common-services/HDFS/2.1.0.2.0/package/scripts/datanode_upgrade.py 8f36001 ambari-server/src/main/resources/common-services/HDFS/2.1.0.2.0/package/scripts/params_linux.py 7ad9f39 ambari-server/src/test/python/stacks/2.0.6/HBASE/test_hbase_regionserver.py 8d187ec ambari-server/src/test/python/stacks/2.0.6/HDFS/test_datanode.py 78b8171 Diff: https://reviews.apache.org/r/46434/diff/ Testing --- I did manual testing on this: For RegionServer the process check is tested. For DataNodes I made an intentional exception to see if it keeps waiting. (this is how I ran into the 20 minutes server task timeout) -- Total run:970 Total errors:0 Total failures:0 OK Thanks, Daniel Gergely
Re: Review Request 46496: Host_status stuck in UNKNOWN status after blueprint deploy with host in heartbeat-lost
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46496/#review129905 --- Fix it, then Ship it! ambari-server/src/main/java/org/apache/ambari/server/topology/TopologyManager.java (line 51) <https://reviews.apache.org/r/46496/#comment193457> I cannot see where HostState is used. ambari-server/src/main/java/org/apache/ambari/server/topology/TopologyManager.java (lines 394 - 397) <https://reviews.apache.org/r/46496/#comment193456> isEmpty check is not necessary here, contains is enough. Even the whole if statement is needed only for logging. So remove operation can be executed directly if log message is not misleading in the case when availableHosts does not contain host. (is it possible?) - Daniel Gergely On ápr. 21, 2016, 3:19 du, Sebastian Toader wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/46496/ > --- > > (Updated ápr. 21, 2016, 3:19 du) > > > Review request for Ambari, Daniel Gergely, Laszlo Puskas, Sandor Magyari, > Sumit Mohanty, and Sid Wagle. > > > Bugs: AMBARI-16013 > https://issues.apache.org/jira/browse/AMBARI-16013 > > > Repository: ambari > > > Description > --- > > When hosts register to Ambari server the `TopologyManager` adds these to its > `availableHosts` collection. When a cluster is provisioned using Blueprints > `TopologyManager` tries to allocate required hosts to hostgroups from the > available hosts collection. In case hosts turned into HEARTBEAT_LOST state > these were not removed from `availableHosts` this resulting scheduling > logical tasks to unreachable hosts. When these unreachable hosts become > available re-register with Ambari server. The server since already scheduled > logical tasks for these it won't try again thus will never create role > commands to be executed by the hosts. > > `TopologyManager` has been hooked now to the HEARTBEAT_LOST state transition > to remove the host in question from its internal `availableHosts` collection. > > > Diffs > - > > > ambari-server/src/main/java/org/apache/ambari/server/state/host/HostImpl.java > d221112 > > ambari-server/src/main/java/org/apache/ambari/server/topology/TopologyManager.java > 5a0aca0 > > Diff: https://reviews.apache.org/r/46496/diff/ > > > Testing > --- > > Manual testign with a 5 node cluster using Blueprints. > > Unit tests: > Results : > > Tests run: 3561, Failures: 0, Errors: 0, Skipped: 36 > > > Thanks, > > Sebastian Toader > >
Re: Review Request 46434: DataNode and RegionServer during upgrade are reported as "failed" incorrectly
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46434/ --- (Updated ápr. 21, 2016, 12:36 du) Review request for Ambari, Alejandro Fernandez, Miklos Gergely, Oliver Szabo, Sandor Magyari, and Sebastian Toader. Changes --- Review fix Bugs: AMBARI-15991 https://issues.apache.org/jira/browse/AMBARI-15991 Repository: ambari Description --- If upgrade process takes longer than expected, DataNode and RegionServer is reported as failed. It happens because it needs more time to finish update. The fix for RegionServer checks if the process is running and if it is so, then it is not considered as a failure. For DataNode the process is also checked and if it is running then check is repeated 2 times with 5 minutes wait. I had a limitation here, python scripts are allowed to run for 20 minutes by default and this checking takes 16 mins (2 minutes initial check, 5 minutes sleep if there is a failure, 2 minutes regaular check, 5 minutes sleep, 2 minutes final check). If more time is needed, then default value of *server.task.timeout* and number of repetition in 5 minutes check should be increased. Diffs (updated) - ambari-server/src/main/resources/common-services/HBASE/0.96.0.2.0/package/scripts/upgrade.py 01a8156 ambari-server/src/main/resources/common-services/HDFS/2.1.0.2.0/package/scripts/datanode_upgrade.py 8f36001 ambari-server/src/main/resources/common-services/HDFS/2.1.0.2.0/package/scripts/params_linux.py 7ad9f39 ambari-server/src/test/python/stacks/2.0.6/HDFS/test_datanode.py 78b8171 Diff: https://reviews.apache.org/r/46434/diff/ Testing --- I did manual testing on this: For RegionServer the process check is tested. For DataNodes I made an intentional exception to see if it keeps waiting. (this is how I ran into the 20 minutes server task timeout) -- Total run:970 Total errors:0 Total failures:0 OK Thanks, Daniel Gergely
Review Request 46434: DataNode and RegionServer during upgrade are reported as "failed" incorrectly
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46434/ --- Review request for Ambari, Alejandro Fernandez, Miklos Gergely, Oliver Szabo, Sandor Magyari, and Sebastian Toader. Bugs: AMBARI-15991 https://issues.apache.org/jira/browse/AMBARI-15991 Repository: ambari Description --- If upgrade process takes longer than expected, DataNode and RegionServer is reported as failed. It happens because it needs more time to finish update. The fix for RegionServer checks if the process is running and if it is so, then it is not considered as a failure. For DataNode the process is also checked and if it is running then check is repeated 2 times with 5 minutes wait. I had a limitation here, python scripts are allowed to run for 20 minutes by default and this checking takes 16 mins (2 minutes initial check, 5 minutes sleep if there is a failure, 2 minutes regaular check, 5 minutes sleep, 2 minutes final check). If more time is needed, then default value of *server.task.timeout* and number of repetition in 5 minutes check should be increased. Diffs - ambari-server/src/main/resources/common-services/HBASE/0.96.0.2.0/package/scripts/upgrade.py 01a8156 ambari-server/src/main/resources/common-services/HDFS/2.1.0.2.0/package/scripts/datanode_upgrade.py 8f36001 ambari-server/src/main/resources/common-services/HDFS/2.1.0.2.0/package/scripts/params_linux.py 7ad9f39 ambari-server/src/test/python/stacks/2.0.6/HDFS/test_datanode.py 78b8171 Diff: https://reviews.apache.org/r/46434/diff/ Testing --- I did manual testing on this: For RegionServer the process check is tested. For DataNodes I made an intentional exception to see if it keeps waiting. (this is how I ran into the 20 minutes server task timeout) -- Total run:970 Total errors:0 Total failures:0 OK Thanks, Daniel Gergely
Re: Review Request 46403: Blueprint processor does not replace "localhost" in "xasecure.audit.destination.hdfs.dir" property.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46403/#review129707 --- Ship it! Ship It! - Daniel Gergely On ápr. 19, 2016, 8:16 du, Sebastian Toader wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/46403/ > --- > > (Updated ápr. 19, 2016, 8:16 du) > > > Review request for Ambari, Balázs Bence Sári, Daniel Gergely, Robert > Nettleton, and Sandor Magyari. > > > Bugs: AMBARI-15978 > https://issues.apache.org/jira/browse/AMBARI-15978 > > > Repository: ambari > > > Description > --- > > Add Blueprint processor property updaters for > `xasecure.audit.destination.hdfs.dir` property for all configuration types > where this propery appears. This property is used when Ranger is configured > to store it's audit logs in HDFS > > > Diffs > - > > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java > 5e8241b > > ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessorTest.java > dd26c75 > > Diff: https://reviews.apache.org/r/46403/diff/ > > > Testing > --- > > Created cluster using Blueprint having Ranger configured to store its audit > logs in HDFS. > > Unit test results: > ViewRegistry.readViewArchive(/var/folders/l6/1zwz7jr910gd0kpykwdr_m9wgn/T/test_dir/file.jar): > expected: 1, actual: 0 > > Tests run: 4226, Failures: 1, Errors: 0, Skipped: 32 > > > The failing `ViewRegistry.readViewArchive` is unrelated to this change. > > > Thanks, > > Sebastian Toader > >
Re: Review Request 46376: Recommendation returns OK response if stack_advisor.py has SyntaxError
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46376/ --- (Updated ápr. 19, 2016, 8:43 de) Review request for Ambari, Laszlo Puskas, Oliver Szabo, Sumit Mohanty, and Srimanth Gunturi. Bugs: AMBARI-15965 https://issues.apache.org/jira/browse/AMBARI-15965 Repository: ambari Description --- In the case of having a syntax error in a stack_advisor.py, it is swallowed and call is returned with exit code 0. This means that configuration changes before the syntax error are applied, but changes after the syntax error are skipped. As a result the browser call returns with the response of 200 OK and configuration that looks to be correct, but it might be incorrect, since parts of the stack advisor is skipped. The fix replaces the general exception to IOError. So only IO related exceptions are swallowed, for example: FileNotFound. This should be swallowed, because if a stack_advisor.py file is missing, but no other file would use it, then it works correctly. (consider the case when the latest stack version has no stack_advisor.py file, since the previous version covers all the functionality that is needed for the latest version) UI will not warn the user if there is an error, so users need to review if there is an error in the recommendations. Diffs - ambari-server/src/main/resources/scripts/stack_advisor.py cdd9acb Diff: https://reviews.apache.org/r/46376/diff/ Testing (updated) --- Manual testing of: - creating syntax error - creating semantical error (incorrect array indexing) - throwing an exception - removing a stack advisor file that is used in the next version (this leads to an error, correctly) - removing a stack advisor file that is NOT used in the next version (no error, correctly) --- Total run:963 Total errors:0 Total failures:0 OK Thanks, Daniel Gergely
Re: Review Request 46148: NPE when deleting a host
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46148/ --- (Updated ápr. 14, 2016, 11:41 de) Review request for Ambari, Laszlo Puskas, Oliver Szabo, Robert Levas, Sandor Magyari, and Sebastian Toader. Bugs: AMBARI-15862 https://issues.apache.org/jira/browse/AMBARI-15862 Repository: ambari Description --- When deleting a host, NPE may occur. TopologyHostRequests are created with hostName=null at the first time, and when hosts are registered, then these null values are replaced with the correct host names. In the case when a host is being deleted, a loop checks the host name for all the requests, and if there is a request with hostName=null, a NPE is thrown. Swapping the sides of **equals** method can solve the problem, since the variable **hostname** is checked before and not null. Lob and Basic annotations were removed, because that column in the database is not a CLOB. This turned out during running the test (in memory db is created based on these annotations). The new test case adds hostRequests to the cluster, some of them have no host name (=null) to simulate hosts that have not registered to the cluster yet. If no exception is thrown when deleting a registered host, then it is successful. Diffs - ambari-server/src/main/java/org/apache/ambari/server/orm/entities/TopologyHostGroupEntity.java 0a81286 ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ClustersImpl.java c5554f7 ambari-server/src/test/java/org/apache/ambari/server/state/cluster/ClustersTest.java daf8d12 Diff: https://reviews.apache.org/r/46148/diff/ Testing (updated) --- Branch 2.2.2 Results : Tests run: 3553, Failures: 0, Errors: 0, Skipped: 36 ... Total run:925 Total errors:0 Total failures:0 ... [INFO] [INFO] BUILD SUCCESS [INFO] [INFO] Total time: 1:11:47.762s [INFO] Finished at: Thu Apr 14 13:40:06 CEST 2016 [INFO] Final Memory: 46M/1060M [INFO] Thanks, Daniel Gergely
Re: Review Request 46148: NPE when deleting a host
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46148/ --- (Updated ápr. 14, 2016, 10:25 de) Review request for Ambari, Laszlo Puskas, Oliver Szabo, Robert Levas, Sandor Magyari, and Sebastian Toader. Changes --- Test added Bugs: AMBARI-15862 https://issues.apache.org/jira/browse/AMBARI-15862 Repository: ambari Description (updated) --- When deleting a host, NPE may occur. TopologyHostRequests are created with hostName=null at the first time, and when hosts are registered, then these null values are replaced with the correct host names. In the case when a host is being deleted, a loop checks the host name for all the requests, and if there is a request with hostName=null, a NPE is thrown. Swapping the sides of **equals** method can solve the problem, since the variable **hostname** is checked before and not null. Lob and Basic annotations were removed, because that column in the database is not a CLOB. This turned out during running the test (in memory db is created based on these annotations). Diffs (updated) - ambari-server/src/main/java/org/apache/ambari/server/orm/entities/TopologyHostGroupEntity.java 0a81286 ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ClustersImpl.java c5554f7 ambari-server/src/test/java/org/apache/ambari/server/state/cluster/ClustersTest.java daf8d12 Diff: https://reviews.apache.org/r/46148/diff/ Testing --- Thanks, Daniel Gergely
Re: Review Request 46148: NPE when deleting a host
> On ápr. 13, 2016, 3:05 du, Oliver Szabo wrote: > > ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ClustersImpl.java, > > line 914 > > <https://reviews.apache.org/r/46148/diff/1/?file=1342675#file1342675line914> > > > > Can you provide a test for that case when the hostname is null? (to > > avoid that someone change the code here again) Yes, will do, but ClustersImpl has no test at all right now, so it will take a while to build up a testcase for that. - Daniel --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46148/#review128689 ------- On ápr. 13, 2016, 3:04 du, Daniel Gergely wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/46148/ > --- > > (Updated ápr. 13, 2016, 3:04 du) > > > Review request for Ambari, Laszlo Puskas, Oliver Szabo, Sandor Magyari, and > Sebastian Toader. > > > Bugs: AMBARI-15862 > https://issues.apache.org/jira/browse/AMBARI-15862 > > > Repository: ambari > > > Description > --- > > When deleting a host, NPE may occur. > TopologyHostRequests are created with hostName=null at the first time, and > when hosts are registered, then these null values are replaced with the > correct host names. > In the case when a host is being deleted, a loop checks the host name for all > the requests, and if there is a request with hostName=null, a NPE is thrown. > Swapping the sides of **equals** method can solve the problem, since the > variable **hostname** is checked before and not null. > > > Diffs > - > > > ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ClustersImpl.java > caab7df > > Diff: https://reviews.apache.org/r/46148/diff/ > > > Testing > --- > > > Thanks, > > Daniel Gergely > >
Re: Review Request 46021: Audit logging cleanup and tests
/RecomendationIgnoreEventCreatorTest.java PRE-CREATION ambari-server/src/test/java/org/apache/ambari/server/audit/request/creator/RepositoryEventCreatorTest.java PRE-CREATION ambari-server/src/test/java/org/apache/ambari/server/audit/request/creator/RepositoryVersionEventCreatorTest.java PRE-CREATION ambari-server/src/test/java/org/apache/ambari/server/audit/request/creator/RequestEventCreatorTest.java PRE-CREATION ambari-server/src/test/java/org/apache/ambari/server/audit/request/creator/ServiceConfigDownloadEventCreatorTest.java PRE-CREATION ambari-server/src/test/java/org/apache/ambari/server/audit/request/creator/ServiceEventCreatorTest.java PRE-CREATION ambari-server/src/test/java/org/apache/ambari/server/audit/request/creator/UnauthorizedEventCreatorTest.java PRE-CREATION ambari-server/src/test/java/org/apache/ambari/server/audit/request/creator/UpgradeEventCreatorTest.java PRE-CREATION ambari-server/src/test/java/org/apache/ambari/server/audit/request/creator/UpgradeItemEventCreatorTest.java PRE-CREATION ambari-server/src/test/java/org/apache/ambari/server/audit/request/creator/UserEventCreatorTest.java PRE-CREATION ambari-server/src/test/java/org/apache/ambari/server/audit/request/creator/ValidationIgnoreEventCreatorTest.java PRE-CREATION ambari-server/src/test/java/org/apache/ambari/server/audit/request/creator/ViewInstanceEventCreatorTest.java PRE-CREATION ambari-server/src/test/java/org/apache/ambari/server/audit/request/creator/ViewPrivilegeEventCreatorTest.java PRE-CREATION Diff: https://reviews.apache.org/r/46021/diff/ Testing --- (the failing test is not related to any of my changes) Results : Failed tests: ViewDirectoryWatcherTest.testDirectoryExtractionOnFileAdd:111 Expectation failure on verify: ViewRegistry.readViewArchive(/var/folders/pj/m3fkd7v172dg2cx82nrsq2zhgn/T/test_dir/file.jar): expected: 1, actual: 0 Tests run: 4202, Failures: 1, Errors: 0, Skipped: 32 Thanks, Daniel Gergely
Re: Review Request 46021: Audit logging cleanup and tests
/RecomendationIgnoreEventCreatorTest.java PRE-CREATION ambari-server/src/test/java/org/apache/ambari/server/audit/request/creator/RepositoryEventCreatorTest.java PRE-CREATION ambari-server/src/test/java/org/apache/ambari/server/audit/request/creator/RepositoryVersionEventCreatorTest.java PRE-CREATION ambari-server/src/test/java/org/apache/ambari/server/audit/request/creator/RequestEventCreatorTest.java PRE-CREATION ambari-server/src/test/java/org/apache/ambari/server/audit/request/creator/ServiceConfigDownloadEventCreatorTest.java PRE-CREATION ambari-server/src/test/java/org/apache/ambari/server/audit/request/creator/ServiceEventCreatorTest.java PRE-CREATION ambari-server/src/test/java/org/apache/ambari/server/audit/request/creator/UnauthorizedEventCreatorTest.java PRE-CREATION ambari-server/src/test/java/org/apache/ambari/server/audit/request/creator/UpgradeEventCreatorTest.java PRE-CREATION ambari-server/src/test/java/org/apache/ambari/server/audit/request/creator/UpgradeItemEventCreatorTest.java PRE-CREATION ambari-server/src/test/java/org/apache/ambari/server/audit/request/creator/UserEventCreatorTest.java PRE-CREATION ambari-server/src/test/java/org/apache/ambari/server/audit/request/creator/ValidationIgnoreEventCreatorTest.java PRE-CREATION ambari-server/src/test/java/org/apache/ambari/server/audit/request/creator/ViewInstanceEventCreatorTest.java PRE-CREATION ambari-server/src/test/java/org/apache/ambari/server/audit/request/creator/ViewPrivilegeEventCreatorTest.java PRE-CREATION Diff: https://reviews.apache.org/r/46021/diff/ Testing --- (the failing test is not related to any of my changes) Results : Failed tests: ViewDirectoryWatcherTest.testDirectoryExtractionOnFileAdd:111 Expectation failure on verify: ViewRegistry.readViewArchive(/var/folders/pj/m3fkd7v172dg2cx82nrsq2zhgn/T/test_dir/file.jar): expected: 1, actual: 0 Tests run: 4202, Failures: 1, Errors: 0, Skipped: 32 Thanks, Daniel Gergely
Re: Review Request 46021: Audit logging cleanup and tests
/RecomendationIgnoreEventCreatorTest.java PRE-CREATION ambari-server/src/test/java/org/apache/ambari/server/audit/request/creator/RepositoryEventCreatorTest.java PRE-CREATION ambari-server/src/test/java/org/apache/ambari/server/audit/request/creator/RepositoryVersionEventCreatorTest.java PRE-CREATION ambari-server/src/test/java/org/apache/ambari/server/audit/request/creator/RequestEventCreatorTest.java PRE-CREATION ambari-server/src/test/java/org/apache/ambari/server/audit/request/creator/ServiceConfigDownloadEventCreatorTest.java PRE-CREATION ambari-server/src/test/java/org/apache/ambari/server/audit/request/creator/ServiceEventCreatorTest.java PRE-CREATION ambari-server/src/test/java/org/apache/ambari/server/audit/request/creator/UnauthorizedEventCreatorTest.java PRE-CREATION ambari-server/src/test/java/org/apache/ambari/server/audit/request/creator/UpgradeEventCreatorTest.java PRE-CREATION ambari-server/src/test/java/org/apache/ambari/server/audit/request/creator/UpgradeItemEventCreatorTest.java PRE-CREATION ambari-server/src/test/java/org/apache/ambari/server/audit/request/creator/UserEventCreatorTest.java PRE-CREATION ambari-server/src/test/java/org/apache/ambari/server/audit/request/creator/ValidationIgnoreEventCreatorTest.java PRE-CREATION ambari-server/src/test/java/org/apache/ambari/server/audit/request/creator/ViewInstanceEventCreatorTest.java PRE-CREATION ambari-server/src/test/java/org/apache/ambari/server/audit/request/creator/ViewPrivilegeEventCreatorTest.java PRE-CREATION Diff: https://reviews.apache.org/r/46021/diff/ Testing --- (the failing test is not related to any of my changes) Results : Failed tests: ViewDirectoryWatcherTest.testDirectoryExtractionOnFileAdd:111 Expectation failure on verify: ViewRegistry.readViewArchive(/var/folders/pj/m3fkd7v172dg2cx82nrsq2zhgn/T/test_dir/file.jar): expected: 1, actual: 0 Tests run: 4202, Failures: 1, Errors: 0, Skipped: 32 Thanks, Daniel Gergely
Re: Review Request 46021: Audit logging cleanup and tests
ambari-server/src/test/java/org/apache/ambari/server/audit/request/creator/UnauthorizedEventCreatorTest.java PRE-CREATION ambari-server/src/test/java/org/apache/ambari/server/audit/request/creator/UpgradeEventCreatorTest.java PRE-CREATION ambari-server/src/test/java/org/apache/ambari/server/audit/request/creator/UpgradeItemEventCreatorTest.java PRE-CREATION ambari-server/src/test/java/org/apache/ambari/server/audit/request/creator/UserEventCreatorTest.java PRE-CREATION ambari-server/src/test/java/org/apache/ambari/server/audit/request/creator/ValidationIgnoreEventCreatorTest.java PRE-CREATION ambari-server/src/test/java/org/apache/ambari/server/audit/request/creator/ViewInstanceEventCreatorTest.java PRE-CREATION ambari-server/src/test/java/org/apache/ambari/server/audit/request/creator/ViewPrivilegeEventCreatorTest.java PRE-CREATION Diff: https://reviews.apache.org/r/46021/diff/ Testing (updated) --- (the failing test is not related to any of my changes) Results : Failed tests: ViewDirectoryWatcherTest.testDirectoryExtractionOnFileAdd:111 Expectation failure on verify: ViewRegistry.readViewArchive(/var/folders/pj/m3fkd7v172dg2cx82nrsq2zhgn/T/test_dir/file.jar): expected: 1, actual: 0 Tests run: 4202, Failures: 1, Errors: 0, Skipped: 32 Thanks, Daniel Gergely
Re: Review Request 46021: Audit logging cleanup and tests
/audit/request/creator/ServiceEventCreatorTest.java PRE-CREATION ambari-server/src/test/java/org/apache/ambari/server/audit/request/creator/UnauthorizedEventCreatorTest.java PRE-CREATION ambari-server/src/test/java/org/apache/ambari/server/audit/request/creator/UpgradeEventCreatorTest.java PRE-CREATION ambari-server/src/test/java/org/apache/ambari/server/audit/request/creator/UpgradeItemEventCreatorTest.java PRE-CREATION ambari-server/src/test/java/org/apache/ambari/server/audit/request/creator/UserEventCreatorTest.java PRE-CREATION ambari-server/src/test/java/org/apache/ambari/server/audit/request/creator/ValidationIgnoreEventCreatorTest.java PRE-CREATION ambari-server/src/test/java/org/apache/ambari/server/audit/request/creator/ViewInstanceEventCreatorTest.java PRE-CREATION ambari-server/src/test/java/org/apache/ambari/server/audit/request/creator/ViewPrivilegeEventCreatorTest.java PRE-CREATION Diff: https://reviews.apache.org/r/46021/diff/ Testing --- Still running locally... Thanks, Daniel Gergely
Review Request 46021: Audit logging cleanup and tests
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46021/ --- Review request for Ambari, Laszlo Puskas, Oliver Szabo, Sandor Magyari, and Sebastian Toader. Bugs: AMBARI-15804 https://issues.apache.org/jira/browse/AMBARI-15804 Repository: ambari Description --- Audit logging cleanup and tests Unit tests for creators Adding component to host now handles multiple components Fixing hostname for host event Diffs - ambari-server/src/main/java/org/apache/ambari/server/audit/event/request/ActivateUserRequestAuditEvent.java df5726d ambari-server/src/main/java/org/apache/ambari/server/audit/event/request/AddComponentToHostRequestAuditEvent.java 2c9eedd ambari-server/src/main/java/org/apache/ambari/server/audit/request/eventcreator/HostEventCreator.java 7c27d19 ambari-server/src/test/java/org/apache/ambari/server/audit/request/TaskStatusAuditEventTest.java PRE-CREATION ambari-server/src/test/java/org/apache/ambari/server/audit/request/creator/AlertGroupEventCreatorTest.java PRE-CREATION ambari-server/src/test/java/org/apache/ambari/server/audit/request/creator/AlertTargetEventCreatorTest.java PRE-CREATION ambari-server/src/test/java/org/apache/ambari/server/audit/request/creator/AuditEventCreatorTestBase.java PRE-CREATION ambari-server/src/test/java/org/apache/ambari/server/audit/request/creator/AuditEventCreatorTestHelper.java PRE-CREATION ambari-server/src/test/java/org/apache/ambari/server/audit/request/creator/BlueprintEventCreatorTest.java PRE-CREATION ambari-server/src/test/java/org/apache/ambari/server/audit/request/creator/BlueprintExportEventCreatorTest.java PRE-CREATION ambari-server/src/test/java/org/apache/ambari/server/audit/request/creator/ComponentEventCreatorTest.java PRE-CREATION ambari-server/src/test/java/org/apache/ambari/server/audit/request/creator/ConfigurationChangeEventCreatorTest.java PRE-CREATION ambari-server/src/test/java/org/apache/ambari/server/audit/request/creator/CredentialEventCreatorTest.java PRE-CREATION ambari-server/src/test/java/org/apache/ambari/server/audit/request/creator/DefaultEventCreatorTest.java PRE-CREATION ambari-server/src/test/java/org/apache/ambari/server/audit/request/creator/GroupEventCreatorTest.java PRE-CREATION ambari-server/src/test/java/org/apache/ambari/server/audit/request/creator/HostEventCreatorTest.java PRE-CREATION ambari-server/src/test/java/org/apache/ambari/server/audit/request/creator/MemberEventCreatorTest.java PRE-CREATION ambari-server/src/test/java/org/apache/ambari/server/audit/request/creator/PrivilegeEventCreatorTest.java PRE-CREATION ambari-server/src/test/java/org/apache/ambari/server/audit/request/creator/RecomendationIgnoreEventCreatorTest.java PRE-CREATION ambari-server/src/test/java/org/apache/ambari/server/audit/request/creator/RepositoryEventCreatorTest.java PRE-CREATION ambari-server/src/test/java/org/apache/ambari/server/audit/request/creator/RepositoryVersionEventCreatorTest.java PRE-CREATION ambari-server/src/test/java/org/apache/ambari/server/audit/request/creator/RequestEventCreatorTest.java PRE-CREATION ambari-server/src/test/java/org/apache/ambari/server/audit/request/creator/ServiceConfigDownloadEventCreatorTest.java PRE-CREATION ambari-server/src/test/java/org/apache/ambari/server/audit/request/creator/ServiceEventCreatorTest.java PRE-CREATION ambari-server/src/test/java/org/apache/ambari/server/audit/request/creator/UnauthorizedEventCreatorTest.java PRE-CREATION ambari-server/src/test/java/org/apache/ambari/server/audit/request/creator/UpgradeEventCreatorTest.java PRE-CREATION ambari-server/src/test/java/org/apache/ambari/server/audit/request/creator/UpgradeItemEventCreatorTest.java PRE-CREATION ambari-server/src/test/java/org/apache/ambari/server/audit/request/creator/UserEventCreatorTest.java PRE-CREATION ambari-server/src/test/java/org/apache/ambari/server/audit/request/creator/ValidationIgnoreEventCreatorTest.java PRE-CREATION ambari-server/src/test/java/org/apache/ambari/server/audit/request/creator/ViewInstanceEventCreatorTest.java PRE-CREATION ambari-server/src/test/java/org/apache/ambari/server/audit/request/creator/ViewPrivilegeEventCreatorTest.java PRE-CREATION Diff: https://reviews.apache.org/r/46021/diff/ Testing --- Still running locally... Thanks, Daniel Gergely
Re: Review Request 45538: Audit Log Code Cleanup & Safety
entcreator/ServiceConfigDownloadEventCreator.java 0999010 ambari-server/src/main/java/org/apache/ambari/server/audit/request/eventcreator/ServiceEventCreator.java 65d94f9 ambari-server/src/main/java/org/apache/ambari/server/audit/request/eventcreator/UnauthorizedEventCreator.java db3c934 ambari-server/src/main/java/org/apache/ambari/server/audit/request/eventcreator/UpgradeEventCreator.java f9f4152 ambari-server/src/main/java/org/apache/ambari/server/audit/request/eventcreator/UpgradeItemEventCreator.java 1869909 ambari-server/src/main/java/org/apache/ambari/server/audit/request/eventcreator/UserEventCreator.java 89f0755 ambari-server/src/main/java/org/apache/ambari/server/audit/request/eventcreator/ValidationIgnoreEventCreator.java 081f3d3 ambari-server/src/main/java/org/apache/ambari/server/audit/request/eventcreator/ViewInstanceEventCreator.java a9a3fcd ambari-server/src/main/java/org/apache/ambari/server/audit/request/eventcreator/ViewPrivilegeEventCreator.java d2d7bd9 ambari-server/src/main/java/org/apache/ambari/server/cleanup/ClasspathScannerUtils.java 4c12a62 ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertGroupResourceProvider.java 36469c1 ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertTargetResourceProvider.java 992d33f ambari-server/src/main/java/org/apache/ambari/server/controller/internal/GroupResourceProvider.java 1678931 ambari-server/src/main/java/org/apache/ambari/server/controller/internal/HostComponentResourceProvider.java 3c33a23 ambari-server/src/main/java/org/apache/ambari/server/controller/internal/MemberResourceProvider.java 04e5f67 ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeItemResourceProvider.java a45b1ac ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeResourceProvider.java 0384f6c ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UserResourceProvider.java fee1826 ambari-server/src/main/java/org/apache/ambari/server/notifications/dispatchers/EmailDispatcher.java 9c2b42b ambari-server/src/main/java/org/apache/ambari/server/security/authentication/AmbariAuthenticationFilter.java 5663ed2 ambari-server/src/main/java/org/apache/ambari/server/security/authorization/AmbariAuthorizationFilter.java 96d6131 ambari-server/src/main/java/org/apache/ambari/server/security/authorization/PermissionHelper.java ecf2d7a ambari-server/src/main/java/org/apache/ambari/server/state/services/AlertNoticeDispatchService.java 0b84568 ambari-server/src/test/java/org/apache/ambari/server/agent/TestHeartbeatHandler.java 3976c03 ambari-server/src/test/java/org/apache/ambari/server/audit/ActionDBAAccessorAuditlogTest.java PRE-CREATION ambari-server/src/test/java/org/apache/ambari/server/audit/OperationStatusAuditEventTest.java 0d2e710 ambari-server/src/test/java/org/apache/ambari/server/audit/request/AbstractBaseCreator.java 02ecb00 ambari-server/src/test/java/org/apache/ambari/server/audit/request/RequestAuditLogModule.java 52ad44c ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerTest.java 05f3dcf ambari-server/src/test/java/org/apache/ambari/server/controller/internal/UpgradeResourceProviderTest.java 4a474bf ambari-server/src/test/java/org/apache/ambari/server/orm/InMemoryDefaultTestModule.java b1336de ambari-server/src/test/java/org/apache/ambari/server/security/authentication/AmbariAuthenticationFilterTest.java f6a885d ambari-server/src/test/java/org/apache/ambari/server/security/authorization/AmbariAuthorizationFilterTest.java 3dd6b0a Diff: https://reviews.apache.org/r/45538/diff/ Testing --- Thanks, Daniel Gergely
Re: Review Request 45538: Audit Log Code Cleanup & Safety
eator.java 681cfb8 ambari-server/src/main/java/org/apache/ambari/server/audit/request/eventcreator/ServiceEventCreator.java 2e2b91d ambari-server/src/main/java/org/apache/ambari/server/audit/request/eventcreator/UnauthorizedEventCreator.java d53aa68 ambari-server/src/main/java/org/apache/ambari/server/audit/request/eventcreator/UpgradeEventCreator.java b8a6873 ambari-server/src/main/java/org/apache/ambari/server/audit/request/eventcreator/UpgradeItemEventCreator.java 9f83172 ambari-server/src/main/java/org/apache/ambari/server/audit/request/eventcreator/UserEventCreator.java 2b4e5c1 ambari-server/src/main/java/org/apache/ambari/server/audit/request/eventcreator/ValidationIgnoreEventCreator.java 081f3d3 ambari-server/src/main/java/org/apache/ambari/server/audit/request/eventcreator/ViewInstanceEventCreator.java 611b1ea ambari-server/src/main/java/org/apache/ambari/server/audit/request/eventcreator/ViewPrivilegeEventCreator.java 18b860a ambari-server/src/main/java/org/apache/ambari/server/cleanup/ClasspathScannerUtils.java 4c12a62 ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertGroupResourceProvider.java 36469c1 ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertTargetResourceProvider.java 992d33f ambari-server/src/main/java/org/apache/ambari/server/controller/internal/GroupResourceProvider.java 1678931 ambari-server/src/main/java/org/apache/ambari/server/controller/internal/HostComponentResourceProvider.java 3c33a23 ambari-server/src/main/java/org/apache/ambari/server/controller/internal/MemberResourceProvider.java 04e5f67 ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeItemResourceProvider.java a45b1ac ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeResourceProvider.java 714495f ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UserResourceProvider.java fee1826 ambari-server/src/main/java/org/apache/ambari/server/notifications/dispatchers/EmailDispatcher.java 9c2b42b ambari-server/src/main/java/org/apache/ambari/server/security/authentication/AmbariAuthenticationFilter.java 5663ed2 ambari-server/src/main/java/org/apache/ambari/server/security/authorization/AmbariAuthorizationFilter.java 96d6131 ambari-server/src/main/java/org/apache/ambari/server/security/authorization/PermissionHelper.java ecf2d7a ambari-server/src/main/java/org/apache/ambari/server/state/services/AlertNoticeDispatchService.java 0b84568 ambari-server/src/test/java/org/apache/ambari/server/audit/ActionDBAAccessorAuditlogTest.java PRE-CREATION ambari-server/src/test/java/org/apache/ambari/server/audit/request/AbstractBaseCreator.java 02ecb00 ambari-server/src/test/java/org/apache/ambari/server/audit/request/RequestAuditLogModule.java 52ad44c ambari-server/src/test/java/org/apache/ambari/server/security/authentication/AmbariAuthenticationFilterTest.java 13636b1 ambari-server/src/test/java/org/apache/ambari/server/security/authorization/AmbariAuthorizationFilterTest.java 3dd6b0a Diff: https://reviews.apache.org/r/45538/diff/ Testing --- Thanks, Daniel Gergely
Re: Review Request 45538: Audit Log Code Cleanup & Safety
> On ápr. 4, 2016, 2:05 du, Jonathan Hurley wrote: > > ambari-server/src/main/java/org/apache/ambari/server/audit/request/eventcreator/AbstractBaseEventCreator.java, > > line 28 > > <https://reviews.apache.org/r/45538/diff/4/?file=132#file132line28> > > > > I'm actually a fan of composition over inheritance. If these methods > > truly are part of a common logic for all Creators, then we can keep it. But > > would it be better to simply have a class which can perform these > > operations and merely reference it from all of the RequestAuditEventCreator > > instances? I don't see any shared state in this class, which is why I ask. I moved out the functionality to a helper class > On ápr. 4, 2016, 2:05 du, Jonathan Hurley wrote: > > ambari-server/src/main/java/org/apache/ambari/server/actionmanager/ActionDBAccessorImpl.java, > > line 847 > > <https://reviews.apache.org/r/45538/diff/4/?file=1322215#file1322215line847> > > > > It look slike this method and the one above can access this cache > > concurrently; you might need to make this Map threadsafe. > > > > Also, it's an interesting approach to clearing this cache. Component > > removals are not very often and I think the original point of clearing the > > cache was just to ensure it didn't stick around forever. > > > > You brought up good points that the cache only was mapping a Long to an > > Enum; maybe a simple expiring cache would have been simpler/safer/better > > here. Guava cache is introduced and it is cleared if a key is not accessed for a certain amount of time (eg 60 minutes). It means that if a request is not checked in the cache for a while, it is cleared. - Daniel --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45538/#review126826 --- On ápr. 4, 2016, 8:24 de, Daniel Gergely wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/45538/ > --- > > (Updated ápr. 4, 2016, 8:24 de) > > > Review request for Ambari, Jonathan Hurley, Nate Cole, and Sebastian Toader. > > > Bugs: AMBARI-15646 > https://issues.apache.org/jira/browse/AMBARI-15646 > > > Repository: ambari > > > Description > --- > > ThreadLocal: > InitialValue() method is used when initializing the ThreadLocal member > variable. > > Multibinder: > The same logic is used for binding multiple classes from a package > "automatically" as in org.apache.ambari.server.cleanup.CleanupModule. > > Creator properties: > Property retrieval from responses now grouped into an abstract baseclass. It > can get properties from namedPropertySets and propertySets. > > Auditlog enabling: > Added checks to more places in the code to skip auditlog related object > creation if auditlog is disabled. > > Cache: > The previously existing 3 variables now groupped into a single data structure > to act as a cache. Every request has a RequestDetails object, which contains > the last status of the request and a map for tasks. A task has a key that is > composed of a component name and a host name, the value is the previous > status of the task. > By using this structure, tasks for components can easily be removed and if > the RequestDetails has no task, the request itself can also be removed. > > > Diffs > - > > > ambari-server/src/main/java/org/apache/ambari/server/actionmanager/ActionDBAccessorImpl.java > 79d3470 > > ambari-server/src/main/java/org/apache/ambari/server/api/services/BaseService.java > 2e5b920 > > ambari-server/src/main/java/org/apache/ambari/server/api/services/LogoutService.java > 3b449ca > > ambari-server/src/main/java/org/apache/ambari/server/audit/AuditLoggerDefaultImpl.java > 1cfb740 > > ambari-server/src/main/java/org/apache/ambari/server/audit/AuditLoggerModule.java > b20714b > > ambari-server/src/main/java/org/apache/ambari/server/audit/event/OperationStatusAuditEvent.java > 319d772 > > ambari-server/src/main/java/org/apache/ambari/server/audit/event/TaskStatusAuditEvent.java > eaea058 > > ambari-server/src/main/java/org/apache/ambari/server/audit/event/request/MembershipChangeRequestAuditEvent.java > 040934e > > ambari-server/src/main/java/org/apache/ambari/server/audit/event/request/StartOperationRequestAuditEvent.java > 66e37b8 > &
Re: Review Request 45538: Audit Log Code Cleanup & Safety
entcreator/UpgradeItemEventCreator.java 9f83172 ambari-server/src/main/java/org/apache/ambari/server/audit/request/eventcreator/UserEventCreator.java 2b4e5c1 ambari-server/src/main/java/org/apache/ambari/server/audit/request/eventcreator/ValidationIgnoreEventCreator.java 081f3d3 ambari-server/src/main/java/org/apache/ambari/server/audit/request/eventcreator/ViewInstanceEventCreator.java 611b1ea ambari-server/src/main/java/org/apache/ambari/server/audit/request/eventcreator/ViewPrivilegeEventCreator.java 18b860a ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertGroupResourceProvider.java 36469c1 ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertTargetResourceProvider.java 992d33f ambari-server/src/main/java/org/apache/ambari/server/controller/internal/GroupResourceProvider.java 1678931 ambari-server/src/main/java/org/apache/ambari/server/controller/internal/HostComponentResourceProvider.java 3c33a23 ambari-server/src/main/java/org/apache/ambari/server/controller/internal/MemberResourceProvider.java 04e5f67 ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeItemResourceProvider.java a45b1ac ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeResourceProvider.java 714495f ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UserResourceProvider.java fee1826 ambari-server/src/main/java/org/apache/ambari/server/notifications/dispatchers/EmailDispatcher.java 9c2b42b ambari-server/src/main/java/org/apache/ambari/server/security/authentication/AmbariAuthenticationFilter.java 5663ed2 ambari-server/src/main/java/org/apache/ambari/server/security/authorization/AmbariAuthorizationFilter.java 96d6131 ambari-server/src/main/java/org/apache/ambari/server/security/authorization/PermissionHelper.java ecf2d7a ambari-server/src/main/java/org/apache/ambari/server/state/services/AlertNoticeDispatchService.java 0b84568 ambari-server/src/test/java/org/apache/ambari/server/audit/request/AbstractBaseCreator.java 02ecb00 ambari-server/src/test/java/org/apache/ambari/server/audit/request/RequestAuditLogModule.java 52ad44c Diff: https://reviews.apache.org/r/45538/diff/ Testing --- Thanks, Daniel Gergely
Review Request 45587: Upgrade to trunk fails due AuditLogger
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45587/ --- Review request for Ambari, Alejandro Fernandez, Oliver Szabo, and Sebastian Toader. Bugs: AMBARI-15669 https://issues.apache.org/jira/browse/AMBARI-15669 Repository: ambari Description --- AuditLoggerModule was missing when injector is created for SchemaUpgradeHelper. AuditLoggerModule is now added to the injector. Diffs - ambari-server/src/main/java/org/apache/ambari/server/upgrade/SchemaUpgradeHelper.java 89e920d Diff: https://reviews.apache.org/r/45587/diff/ Testing --- I did an upgrade from 2.2.2 to trunk to verify the fix. Thanks, Daniel Gergely
Re: Review Request 45538: Audit Log Code Cleanup & Safety
/eventcreator/ViewInstanceEventCreator.java 611b1ea ambari-server/src/main/java/org/apache/ambari/server/audit/request/eventcreator/ViewPrivilegeEventCreator.java 18b860a ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertGroupResourceProvider.java 36469c1 ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertTargetResourceProvider.java 992d33f ambari-server/src/main/java/org/apache/ambari/server/controller/internal/GroupResourceProvider.java 1678931 ambari-server/src/main/java/org/apache/ambari/server/controller/internal/HostComponentResourceProvider.java 3c33a23 ambari-server/src/main/java/org/apache/ambari/server/controller/internal/MemberResourceProvider.java 04e5f67 ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeItemResourceProvider.java a45b1ac ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeResourceProvider.java 714495f ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UserResourceProvider.java fee1826 ambari-server/src/main/java/org/apache/ambari/server/notifications/dispatchers/EmailDispatcher.java 9c2b42b ambari-server/src/main/java/org/apache/ambari/server/security/authentication/AmbariAuthenticationFilter.java 5663ed2 ambari-server/src/main/java/org/apache/ambari/server/security/authorization/AmbariAuthorizationFilter.java 96d6131 ambari-server/src/main/java/org/apache/ambari/server/security/authorization/PermissionHelper.java ecf2d7a ambari-server/src/main/java/org/apache/ambari/server/state/services/AlertNoticeDispatchService.java 0b84568 ambari-server/src/test/java/org/apache/ambari/server/audit/request/AbstractBaseCreator.java 02ecb00 ambari-server/src/test/java/org/apache/ambari/server/audit/request/RequestAuditLogModule.java 52ad44c Diff: https://reviews.apache.org/r/45538/diff/ Testing --- Thanks, Daniel Gergely
Re: Review Request 45579: Unable to Create Cluster Fails Due To Audit Logger
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45579/ --- (Updated ápr. 1, 2016, 12:07 du) Review request for Ambari, Jonathan Hurley, Oliver Szabo, and Sebastian Toader. Bugs: AMBARI-15665 https://issues.apache.org/jira/browse/AMBARI-15665 Repository: ambari Description --- Due to a programming error, username was accuired from directly the Spring Security Context. When the ldap authentication was used, type casting failed. Now username is retrieved by using the AuthorizationHelper class and setting username is moved to AbstractUserAuditEvent. Diffs - ambari-server/src/main/java/org/apache/ambari/server/audit/event/AbstractUserAuditEvent.java a968a64 ambari-server/src/main/java/org/apache/ambari/server/audit/request/eventcreator/AlertGroupEventCreator.java 103fd4d ambari-server/src/main/java/org/apache/ambari/server/audit/request/eventcreator/AlertTargetEventCreator.java 29a241e ambari-server/src/main/java/org/apache/ambari/server/audit/request/eventcreator/BlueprintEventCreator.java bdd6dbe ambari-server/src/main/java/org/apache/ambari/server/audit/request/eventcreator/BlueprintExportEventCreator.java 1416021 ambari-server/src/main/java/org/apache/ambari/server/audit/request/eventcreator/ComponentEventCreator.java 8034d24 ambari-server/src/main/java/org/apache/ambari/server/audit/request/eventcreator/ConfigurationChangeEventCreator.java 7e58893 ambari-server/src/main/java/org/apache/ambari/server/audit/request/eventcreator/CredentialEventCreator.java 3b1f462 ambari-server/src/main/java/org/apache/ambari/server/audit/request/eventcreator/DefaultEventCreator.java d0f57f2 ambari-server/src/main/java/org/apache/ambari/server/audit/request/eventcreator/GroupEventCreator.java d926d94 ambari-server/src/main/java/org/apache/ambari/server/audit/request/eventcreator/HostEventCreator.java 910280d ambari-server/src/main/java/org/apache/ambari/server/audit/request/eventcreator/MemberEventCreator.java a3c3164 ambari-server/src/main/java/org/apache/ambari/server/audit/request/eventcreator/PrivilegeEventCreator.java bdc7b59 ambari-server/src/main/java/org/apache/ambari/server/audit/request/eventcreator/RepositoryEventCreator.java fe6f8cc ambari-server/src/main/java/org/apache/ambari/server/audit/request/eventcreator/RepositoryVersionEventCreator.java 7c9c731 ambari-server/src/main/java/org/apache/ambari/server/audit/request/eventcreator/RequestEventCreator.java fd13973 ambari-server/src/main/java/org/apache/ambari/server/audit/request/eventcreator/ServiceConfigDownloadEventCreator.java 681cfb8 ambari-server/src/main/java/org/apache/ambari/server/audit/request/eventcreator/ServiceEventCreator.java 2e2b91d ambari-server/src/main/java/org/apache/ambari/server/audit/request/eventcreator/UnauthorizedEventCreator.java d53aa68 ambari-server/src/main/java/org/apache/ambari/server/audit/request/eventcreator/UpgradeEventCreator.java b8a6873 ambari-server/src/main/java/org/apache/ambari/server/audit/request/eventcreator/UpgradeItemEventCreator.java 9f83172 ambari-server/src/main/java/org/apache/ambari/server/audit/request/eventcreator/UserEventCreator.java 2b4e5c1 ambari-server/src/main/java/org/apache/ambari/server/audit/request/eventcreator/ViewInstanceEventCreator.java 611b1ea ambari-server/src/main/java/org/apache/ambari/server/audit/request/eventcreator/ViewPrivilegeEventCreator.java 18b860a ambari-server/src/test/java/org/apache/ambari/server/audit/request/DefaultEventCreatorTest.java 5c23059 Diff: https://reviews.apache.org/r/45579/diff/ Testing --- Auditlog and AuthorizationHelper tests should cover the test cases of the current change. Thanks, Daniel Gergely
Re: Review Request 45538: Audit Log Code Cleanup & Safety
/apache/ambari/server/controller/internal/GroupResourceProvider.java 1678931 ambari-server/src/main/java/org/apache/ambari/server/controller/internal/HostComponentResourceProvider.java 3c33a23 ambari-server/src/main/java/org/apache/ambari/server/controller/internal/MemberResourceProvider.java 04e5f67 ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeItemResourceProvider.java a45b1ac ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeResourceProvider.java 714495f ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UserResourceProvider.java fee1826 ambari-server/src/main/java/org/apache/ambari/server/notifications/dispatchers/EmailDispatcher.java 9c2b42b ambari-server/src/main/java/org/apache/ambari/server/security/authentication/AmbariAuthenticationFilter.java 5663ed2 ambari-server/src/main/java/org/apache/ambari/server/security/authorization/AmbariAuthorizationFilter.java 96d6131 ambari-server/src/main/java/org/apache/ambari/server/security/authorization/PermissionHelper.java ecf2d7a ambari-server/src/main/java/org/apache/ambari/server/state/services/AlertNoticeDispatchService.java 0b84568 Diff: https://reviews.apache.org/r/45538/diff/ Testing --- Thanks, Daniel Gergely
Review Request 45579: Unable to Create Cluster Fails Due To Audit Logger
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45579/ --- Review request for Ambari, Jonathan Hurley, Oliver Szabo, and Sebastian Toader. Bugs: AMBARI-15665 https://issues.apache.org/jira/browse/AMBARI-15665 Repository: ambari Description --- Due to a programming error, username was accuired from directly the Spring Security Context. When the ldap authentication was used, type casting failed. Now username is retrieved by using the AuthorizationHelper class and setting username is moved to AbstractUserAuditEvent. Diffs - ambari-server/src/main/java/org/apache/ambari/server/audit/event/AbstractUserAuditEvent.java a968a64 ambari-server/src/main/java/org/apache/ambari/server/audit/request/eventcreator/AlertGroupEventCreator.java 103fd4d ambari-server/src/main/java/org/apache/ambari/server/audit/request/eventcreator/AlertTargetEventCreator.java 29a241e ambari-server/src/main/java/org/apache/ambari/server/audit/request/eventcreator/BlueprintEventCreator.java bdd6dbe ambari-server/src/main/java/org/apache/ambari/server/audit/request/eventcreator/BlueprintExportEventCreator.java 1416021 ambari-server/src/main/java/org/apache/ambari/server/audit/request/eventcreator/ComponentEventCreator.java 8034d24 ambari-server/src/main/java/org/apache/ambari/server/audit/request/eventcreator/ConfigurationChangeEventCreator.java 7e58893 ambari-server/src/main/java/org/apache/ambari/server/audit/request/eventcreator/CredentialEventCreator.java 3b1f462 ambari-server/src/main/java/org/apache/ambari/server/audit/request/eventcreator/DefaultEventCreator.java d0f57f2 ambari-server/src/main/java/org/apache/ambari/server/audit/request/eventcreator/GroupEventCreator.java d926d94 ambari-server/src/main/java/org/apache/ambari/server/audit/request/eventcreator/HostEventCreator.java 910280d ambari-server/src/main/java/org/apache/ambari/server/audit/request/eventcreator/MemberEventCreator.java a3c3164 ambari-server/src/main/java/org/apache/ambari/server/audit/request/eventcreator/PrivilegeEventCreator.java bdc7b59 ambari-server/src/main/java/org/apache/ambari/server/audit/request/eventcreator/RepositoryEventCreator.java fe6f8cc ambari-server/src/main/java/org/apache/ambari/server/audit/request/eventcreator/RepositoryVersionEventCreator.java 7c9c731 ambari-server/src/main/java/org/apache/ambari/server/audit/request/eventcreator/RequestEventCreator.java fd13973 ambari-server/src/main/java/org/apache/ambari/server/audit/request/eventcreator/ServiceConfigDownloadEventCreator.java 681cfb8 ambari-server/src/main/java/org/apache/ambari/server/audit/request/eventcreator/ServiceEventCreator.java 2e2b91d ambari-server/src/main/java/org/apache/ambari/server/audit/request/eventcreator/UnauthorizedEventCreator.java d53aa68 ambari-server/src/main/java/org/apache/ambari/server/audit/request/eventcreator/UpgradeEventCreator.java b8a6873 ambari-server/src/main/java/org/apache/ambari/server/audit/request/eventcreator/UpgradeItemEventCreator.java 9f83172 ambari-server/src/main/java/org/apache/ambari/server/audit/request/eventcreator/UserEventCreator.java 2b4e5c1 ambari-server/src/main/java/org/apache/ambari/server/audit/request/eventcreator/ViewInstanceEventCreator.java 611b1ea ambari-server/src/main/java/org/apache/ambari/server/audit/request/eventcreator/ViewPrivilegeEventCreator.java 18b860a ambari-server/src/test/java/org/apache/ambari/server/audit/request/DefaultEventCreatorTest.java 5c23059 Diff: https://reviews.apache.org/r/45579/diff/ Testing --- Auditlog and AuthorizationHelper tests should cover the test cases of the current change. Thanks, Daniel Gergely
Re: Review Request 45544: During cluster creation using Blueprints the cluster creation request has incorrect COMPLETED state instead of PENDING.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45544/#review126307 --- Ship it! Ship It! - Daniel Gergely On márc. 31, 2016, 2:28 du, Sebastian Toader wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/45544/ > --- > > (Updated márc. 31, 2016, 2:28 du) > > > Review request for Ambari, Daniel Gergely, Oliver Szabo, and Sandor Magyari. > > > Bugs: AMBARI-15643 > https://issues.apache.org/jira/browse/AMBARI-15643 > > > Repository: ambari > > > Description > --- > > *Problem* > *After the cluster creation template posted to Ambari (via the REST Api) to > provision a cluster using Blueprints is accepted for processing Ambari > returns in the response the url of the request through which the > status/progress of the cluster creation can be tracked. > When querying the status of the request it erroneously returns "COMPLETED" > instead of returning "PENDING" until the first task of the cluster creation > is scheduled to be executed on one of the cluster nodes. Once at least one > task is scheduled to be executed the status of the request should change to > "IN_PROGRESS". The "COMPLETED" state should be reached when all tasks > completed succesfully. > > *Resolution* > Make distinction between requests initiated from the UI and Blueprints. If > the request is a request for Blueprint provisioned clusters and there are no > host level tasks available than the request status derivation logic to > default to PENDING. > > > Diffs > - > > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/RequestResourceProvider.java > d00ce58 > > ambari-server/src/test/java/org/apache/ambari/server/controller/internal/RequestResourceProviderTest.java > 4a76a1c > > Diff: https://reviews.apache.org/r/45544/diff/ > > > Testing > --- > > Manual testing and unit tests: > > Results : > > Tests run: 3543, Failures: 0, Errors: 0, Skipped: 36 > > > Thanks, > > Sebastian Toader > >
Re: Review Request 45538: Audit Log Code Cleanup & Safety
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45538/ --- (Updated márc. 31, 2016, 1:51 du) Review request for Ambari, Jonathan Hurley, Nate Cole, and Sebastian Toader. Bugs: AMBARI-15646 https://issues.apache.org/jira/browse/AMBARI-15646 Repository: ambari Description --- ThreadLocal and Cache clearing issues from the ticket. These are open for review. The rest of the issues will be added soon. The previously existing 3 variables now groupped into a single data structure to act as a cache. Every request has a RequestDetails object, which contains the last status of the request and a map for tasks. A task has a key that is composed of a component name and a host name, the value is the previous status of the task. By using this structure, tasks for components can easily be removed and if the RequestDetails has no task, the request itself can also be removed. Diffs (updated) - ambari-server/src/main/java/org/apache/ambari/server/actionmanager/ActionDBAccessorImpl.java 79d3470 ambari-server/src/main/java/org/apache/ambari/server/api/services/LogoutService.java 3b449ca ambari-server/src/main/java/org/apache/ambari/server/audit/AuditLoggerDefaultImpl.java 1cfb740 ambari-server/src/main/java/org/apache/ambari/server/security/authentication/AmbariAuthenticationFilter.java 5663ed2 ambari-server/src/main/java/org/apache/ambari/server/security/authorization/AmbariAuthorizationFilter.java 96d6131 ambari-server/src/main/java/org/apache/ambari/server/security/authorization/PermissionHelper.java ecf2d7a Diff: https://reviews.apache.org/r/45538/diff/ Testing --- Thanks, Daniel Gergely
Review Request 45538: Audit Log Code Cleanup & Safety
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45538/ --- Review request for Ambari, Jonathan Hurley, Nate Cole, and Sebastian Toader. Bugs: AMBARI-15646 https://issues.apache.org/jira/browse/AMBARI-15646 Repository: ambari Description --- ThreadLocal and Cache clearing issues from the ticket. These are open for review. The rest of the issues will be added soon. The previously existing 3 variables now groupped into a single data structure to act as a cache. Every request has a RequestDetails object, which contains the last status of the request and a map for tasks. A task has a key that is composed of a component name and a host name, the value is the previous status of the task. By using this structure, tasks for components can easily be removed and if the RequestDetails has no task, the request itself can also be removed. Diffs - ambari-server/src/main/java/org/apache/ambari/server/actionmanager/ActionDBAccessorImpl.java 79d3470 ambari-server/src/main/java/org/apache/ambari/server/audit/AuditLoggerDefaultImpl.java 1cfb740 ambari-server/src/main/java/org/apache/ambari/server/security/authorization/PermissionHelper.java ecf2d7a Diff: https://reviews.apache.org/r/45538/diff/ Testing --- Thanks, Daniel Gergely
Re: Review Request 44265: Basic Operational Audit Logging
-server/src/main/java/org/apache/ambari/server/security/authentication/AmbariAuthenticationFilter.java PRE-CREATION ambari-server/src/main/java/org/apache/ambari/server/security/authorization/AmbariAuthorizationFilter.java 4be804d ambari-server/src/main/java/org/apache/ambari/server/security/authorization/AuthorizationHelper.java 15ef8d0 ambari-server/src/main/java/org/apache/ambari/server/security/authorization/PermissionHelper.java PRE-CREATION ambari-server/src/main/java/org/apache/ambari/server/serveraction/AbstractServerAction.java 33191bf ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/CreateKeytabFilesServerAction.java 8aa816d ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/CreatePrincipalsServerAction.java 8009ae1 ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/DestroyPrincipalsServerAction.java 93daae8 ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/FinalizeKerberosServerAction.java c710b8e ambari-server/src/main/java/org/apache/ambari/server/utils/RequestUtils.java PRE-CREATION ambari-server/src/main/resources/webapp/WEB-INF/spring-security.xml 8b44b94 ambari-server/src/test/java/org/apache/ambari/server/actionmanager/TestActionDBAccessorImpl.java f88cf8e ambari-server/src/test/java/org/apache/ambari/server/actionmanager/TestActionManager.java d7d08b1 ambari-server/src/test/java/org/apache/ambari/server/agent/HeartbeatProcessorTest.java 7f3d763 ambari-server/src/test/java/org/apache/ambari/server/api/services/BaseServiceTest.java 19eeffb ambari-server/src/test/java/org/apache/ambari/server/audit/AccessUnauthorizedAuditEventTest.java PRE-CREATION ambari-server/src/test/java/org/apache/ambari/server/audit/LoginAuditEventTest.java PRE-CREATION ambari-server/src/test/java/org/apache/ambari/server/audit/LogoutAuditEventTest.java PRE-CREATION ambari-server/src/test/java/org/apache/ambari/server/audit/OperationStatusAuditEventTest.java PRE-CREATION ambari-server/src/test/java/org/apache/ambari/server/audit/StartOperationRequestAuditEventTest.java PRE-CREATION ambari-server/src/test/java/org/apache/ambari/server/audit/request/AbstractBaseCreator.java PRE-CREATION ambari-server/src/test/java/org/apache/ambari/server/audit/request/AllGetCreator.java PRE-CREATION ambari-server/src/test/java/org/apache/ambari/server/audit/request/AllPostAndPutCreator.java PRE-CREATION ambari-server/src/test/java/org/apache/ambari/server/audit/request/DefaultEventCreatorTest.java PRE-CREATION ambari-server/src/test/java/org/apache/ambari/server/audit/request/PutHostComponentCreator.java PRE-CREATION ambari-server/src/test/java/org/apache/ambari/server/audit/request/RequestAuditLogModule.java PRE-CREATION ambari-server/src/test/java/org/apache/ambari/server/audit/request/RequestAuditLoggerTest.java PRE-CREATION ambari-server/src/test/java/org/apache/ambari/server/checks/UpgradeCheckOrderTest.java d4ff566 ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerTest.java ca062c0 ambari-server/src/test/java/org/apache/ambari/server/controller/KerberosHelperTest.java f6027f3 ambari-server/src/test/java/org/apache/ambari/server/notifications/DispatchFactoryTest.java 6834d5c ambari-server/src/test/java/org/apache/ambari/server/orm/InMemoryDefaultTestModule.java 3ecfe14 ambari-server/src/test/java/org/apache/ambari/server/orm/JdbcPropertyTest.java c07382b ambari-server/src/test/java/org/apache/ambari/server/security/authentication/AmbariAuthenticationFilterTest.java PRE-CREATION ambari-server/src/test/java/org/apache/ambari/server/security/authorization/AmbariAuthorizationFilterTest.java 9db3904 ambari-server/src/test/java/org/apache/ambari/server/security/authorization/AmbariLdapAuthenticationProviderForDNWithSpaceTest.java da8d9bc ambari-server/src/test/java/org/apache/ambari/server/security/authorization/AmbariLdapAuthenticationProviderTest.java b26494c ambari-server/src/test/java/org/apache/ambari/server/security/authorization/AmbariLocalUserDetailsServiceTest.java c410f5b ambari-server/src/test/java/org/apache/ambari/server/security/authorization/LdapServerPropertiesTest.java bfb7a90 ambari-server/src/test/java/org/apache/ambari/server/serveraction/kerberos/KerberosServerActionTest.java ce97f25 ambari-server/src/test/java/org/apache/ambari/server/serveraction/kerberos/UpdateKerberosConfigsServerActionTest.java d6f9efe ambari-server/src/test/java/org/apache/ambari/server/utils/RequestUtilsTest.java PRE-CREATION Diff: https://reviews.apache.org/r/44265/diff/ Testing --- Unit tests are added to cover functionality. All tests passed on local machine. Thanks, Daniel Gergely
Re: Review Request 44265: Basic Operational Audit Logging
> On márc. 29, 2016, 4:58 du, Nate Cole wrote: > > ambari-server/src/main/java/org/apache/ambari/server/audit/AuditLoggerModule.java, > > line 65 > > <https://reviews.apache.org/r/44265/diff/4/?file=1313501#file1313501line65> > > > > Can these be bound by annotation? It would make it easier for a > > developer to add creators then to remember to have to come here. Spring framework can do it, but I did not find any similar for guice. - Daniel --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44265/#review125890 ------- On márc. 30, 2016, 12:44 du, Daniel Gergely wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/44265/ > --- > > (Updated márc. 30, 2016, 12:44 du) > > > Review request for Ambari, Laszlo Puskas, Oliver Szabo, Robert Levas, Sandor > Magyari, and Sebastian Toader. > > > Bugs: AMBARI-15241 > https://issues.apache.org/jira/browse/AMBARI-15241 > > > Repository: ambari > > > Description > --- > > Entry points for logging events: > - Login and logout: AmbariAuthenticationFilter, AmbariAuthorizationFilter, > LogoutService > - Operation and tasks: ActionDBAAccessorImpl > - Kerberos related events: CreateKeytabFilesServerAction, > CreatePrincipalServerAction, DestroyPrincipalsServerAction, > FinalizeKerberosServerAction > - REST api: BaseService > > Architecture: > AuditLogger injectable is created and wired in by Guice. The default > implementation (AuditLoggerDefaultImpl) for this interface logs to a file. > Considering performance impact, there is a buffered audit logger > implementation (BufferedAuditLogger) that is a wrapper for an audit logger > implementation and does the logging asynchronously. > > Audit loggers have a single log method that accepts an AuditEvent object. At > the entry points an AuditEvent is assembled and log method is called (except > for the REST api, see below...) > > REST Api: > In BaseService, there is a RequestAuditLogger, that is responsible for > handling and assembling AuditEvents. It also has a log method, but the > parameters for that is the Request and Result objects. > RequestAuditLogger is a small framework for that plugins can be implemented > to handle different type of requests. > Plugins implements RequestAuditEventCreator interface and can be set to > handle one or multiple request types (PUT, POST, DELETE, etc...), resource > types (HOST_COMPONENT, BLUEPRINT, etc..) and results statuses (200 OK, 202 > ACCEPTED, 404 NOT_FOUND, etc). If null is returned by these getters, it means > "all". If there are more than one RequestAuditEventCreators can handle a > request, then the most specific is called. (Priority order: resourceType > > resultStatus > requestType) > For example it is possible to define a plugin for all the POST requests, but > if there is an other plugin for POST request and for resource type > HOST_COMPONENT, then this latter is used (because it is more specific). The > method createAuditEvent is responsible for creating the AuditEvent object for > the RequestAuditEventLogger. If null is returned, then request is not logged > (can be used for ignoring events). > Plugins are registered in ControllerModule and wired by guice to the > RequestAuditLogger. If a new plugin is needed, then implementing the > RequestAuditEventCreator interface and registering it in Guice is the only > things to do. (open-closed principle) > > > Diffs > - > > ambari-project/pom.xml b3d9ca2 > ambari-server/conf/unix/log4j.properties 2ee32d4 > ambari-server/conf/windows/log4j.properties cc40f74 > ambari-server/pom.xml 1e44517 > ambari-server/src/main/conf/log4j.properties 8e6652d > > ambari-server/src/main/java/org/apache/ambari/server/actionmanager/ActionDBAccessorImpl.java > 429f573 > > ambari-server/src/main/java/org/apache/ambari/server/api/services/BaseRequest.java > a33e8a7 > > ambari-server/src/main/java/org/apache/ambari/server/api/services/BaseService.java > 7945599 > > ambari-server/src/main/java/org/apache/ambari/server/api/services/LogoutService.java > df8faaa > > ambari-server/src/main/java/org/apache/ambari/server/api/services/Request.java > ff9b6c7 > > ambari-server/src/main/java/org/apache/ambari/server/audit/AsyncAuditLogger.java > PRE-CREATION > ambari-server/src/main/java/org/apa
Re: Review Request 44265: Basic Operational Audit Logging
> On márc. 29, 2016, 4:33 du, Jonathan Hurley wrote: > > ambari-server/src/main/java/org/apache/ambari/server/actionmanager/ActionDBAccessorImpl.java, > > lines 134-139 > > <https://reviews.apache.org/r/44265/diff/4/?file=1313493#file1313493line134> > > > > I don't see these being cleared anywhere. These are not cleared, because there is no expression to decide when to clear. The problem is that agents sends the completed status multiple times. So if I deleted a completed task from the cache, the agent would send it again. If the task is not in the cache, it is logged, even it is in completed state. (consider the case when ambari server is stopped during a task execution, task finishes and server is started. That point the completed status is needed to be logged, since this is a new information) > On márc. 29, 2016, 4:33 du, Jonathan Hurley wrote: > > ambari-server/src/main/java/org/apache/ambari/server/audit/AuditLoggerDefaultImpl.java, > > line 64 > > <https://reviews.apache.org/r/44265/diff/4/?file=1313500#file1313500line64> > > > > Constructing a new date formatter for every entry? Moved to ThreadLocal. - Daniel --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44265/#review125892 ------- On márc. 30, 2016, 12:44 du, Daniel Gergely wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/44265/ > --- > > (Updated márc. 30, 2016, 12:44 du) > > > Review request for Ambari, Laszlo Puskas, Oliver Szabo, Robert Levas, Sandor > Magyari, and Sebastian Toader. > > > Bugs: AMBARI-15241 > https://issues.apache.org/jira/browse/AMBARI-15241 > > > Repository: ambari > > > Description > --- > > Entry points for logging events: > - Login and logout: AmbariAuthenticationFilter, AmbariAuthorizationFilter, > LogoutService > - Operation and tasks: ActionDBAAccessorImpl > - Kerberos related events: CreateKeytabFilesServerAction, > CreatePrincipalServerAction, DestroyPrincipalsServerAction, > FinalizeKerberosServerAction > - REST api: BaseService > > Architecture: > AuditLogger injectable is created and wired in by Guice. The default > implementation (AuditLoggerDefaultImpl) for this interface logs to a file. > Considering performance impact, there is a buffered audit logger > implementation (BufferedAuditLogger) that is a wrapper for an audit logger > implementation and does the logging asynchronously. > > Audit loggers have a single log method that accepts an AuditEvent object. At > the entry points an AuditEvent is assembled and log method is called (except > for the REST api, see below...) > > REST Api: > In BaseService, there is a RequestAuditLogger, that is responsible for > handling and assembling AuditEvents. It also has a log method, but the > parameters for that is the Request and Result objects. > RequestAuditLogger is a small framework for that plugins can be implemented > to handle different type of requests. > Plugins implements RequestAuditEventCreator interface and can be set to > handle one or multiple request types (PUT, POST, DELETE, etc...), resource > types (HOST_COMPONENT, BLUEPRINT, etc..) and results statuses (200 OK, 202 > ACCEPTED, 404 NOT_FOUND, etc). If null is returned by these getters, it means > "all". If there are more than one RequestAuditEventCreators can handle a > request, then the most specific is called. (Priority order: resourceType > > resultStatus > requestType) > For example it is possible to define a plugin for all the POST requests, but > if there is an other plugin for POST request and for resource type > HOST_COMPONENT, then this latter is used (because it is more specific). The > method createAuditEvent is responsible for creating the AuditEvent object for > the RequestAuditEventLogger. If null is returned, then request is not logged > (can be used for ignoring events). > Plugins are registered in ControllerModule and wired by guice to the > RequestAuditLogger. If a new plugin is needed, then implementing the > RequestAuditEventCreator interface and registering it in Guice is the only > things to do. (open-closed principle) > > > Diffs > - > > ambari-project/pom.xml b3d9ca2 > ambari-server/conf/unix/log4j.properties 2ee32d4 > ambari-server/conf/windows/log4j.properties cc40f74 > ambari-server/pom.xml 1e44517 > ambari-server/src/main/conf/log4j.properties 8
Re: Review Request 44265: Basic Operational Audit Logging
> On márc. 8, 2016, 5:05 du, Jonathan Hurley wrote: > > ambari-server/src/main/java/org/apache/ambari/server/actionmanager/ActionDBAccessorImpl.java, > > lines 801-816 > > <https://reviews.apache.org/r/44265/diff/2/?file=1291527#file1291527line801> > > > > A problem with this approach is that you're building these heavy > > objects even if audit logging is not enabled. Auditlog switch is added, objects are not built if auditlog is disabled. > On márc. 8, 2016, 5:05 du, Jonathan Hurley wrote: > > ambari-server/src/main/java/org/apache/ambari/server/api/services/LogoutService.java, > > line 54 > > <https://reviews.apache.org/r/44265/diff/2/?file=1291530#file1291530line54> > > > > This has to be able to be done via an AOP interceptor. Audit logging is > > a perfect example of how cross cutting concerns can decouple business logic > > from auditting/logging. I dont think we can do it system-wide, because a major refactor would be necessary to have clean pointcuts (e.g. methods with small specific tasks). For example in BaseService the input is url, request, headers, etc and the output is an http Response object. So that would be really difficult to retrieve the information from those. - Daniel --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44265/#review122542 ------- On márc. 30, 2016, 12:44 du, Daniel Gergely wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/44265/ > --- > > (Updated márc. 30, 2016, 12:44 du) > > > Review request for Ambari, Laszlo Puskas, Oliver Szabo, Robert Levas, Sandor > Magyari, and Sebastian Toader. > > > Bugs: AMBARI-15241 > https://issues.apache.org/jira/browse/AMBARI-15241 > > > Repository: ambari > > > Description > --- > > Entry points for logging events: > - Login and logout: AmbariAuthenticationFilter, AmbariAuthorizationFilter, > LogoutService > - Operation and tasks: ActionDBAAccessorImpl > - Kerberos related events: CreateKeytabFilesServerAction, > CreatePrincipalServerAction, DestroyPrincipalsServerAction, > FinalizeKerberosServerAction > - REST api: BaseService > > Architecture: > AuditLogger injectable is created and wired in by Guice. The default > implementation (AuditLoggerDefaultImpl) for this interface logs to a file. > Considering performance impact, there is a buffered audit logger > implementation (BufferedAuditLogger) that is a wrapper for an audit logger > implementation and does the logging asynchronously. > > Audit loggers have a single log method that accepts an AuditEvent object. At > the entry points an AuditEvent is assembled and log method is called (except > for the REST api, see below...) > > REST Api: > In BaseService, there is a RequestAuditLogger, that is responsible for > handling and assembling AuditEvents. It also has a log method, but the > parameters for that is the Request and Result objects. > RequestAuditLogger is a small framework for that plugins can be implemented > to handle different type of requests. > Plugins implements RequestAuditEventCreator interface and can be set to > handle one or multiple request types (PUT, POST, DELETE, etc...), resource > types (HOST_COMPONENT, BLUEPRINT, etc..) and results statuses (200 OK, 202 > ACCEPTED, 404 NOT_FOUND, etc). If null is returned by these getters, it means > "all". If there are more than one RequestAuditEventCreators can handle a > request, then the most specific is called. (Priority order: resourceType > > resultStatus > requestType) > For example it is possible to define a plugin for all the POST requests, but > if there is an other plugin for POST request and for resource type > HOST_COMPONENT, then this latter is used (because it is more specific). The > method createAuditEvent is responsible for creating the AuditEvent object for > the RequestAuditEventLogger. If null is returned, then request is not logged > (can be used for ignoring events). > Plugins are registered in ControllerModule and wired by guice to the > RequestAuditLogger. If a new plugin is needed, then implementing the > RequestAuditEventCreator interface and registering it in Guice is the only > things to do. (open-closed principle) > > > Diffs > - > > ambari-project/pom.xml b3d9ca2 > ambari-server/conf/unix/log4j.properties 2ee32d4 > ambari-server/conf/windows/log4j.properties cc40f7
Re: Review Request 44265: Basic Operational Audit Logging
-server/src/main/java/org/apache/ambari/server/security/authentication/AmbariAuthenticationFilter.java PRE-CREATION ambari-server/src/main/java/org/apache/ambari/server/security/authorization/AmbariAuthorizationFilter.java 4be804d ambari-server/src/main/java/org/apache/ambari/server/security/authorization/AuthorizationHelper.java b136182 ambari-server/src/main/java/org/apache/ambari/server/security/authorization/PermissionHelper.java PRE-CREATION ambari-server/src/main/java/org/apache/ambari/server/serveraction/AbstractServerAction.java 33191bf ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/CreateKeytabFilesServerAction.java cadfe28 ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/CreatePrincipalsServerAction.java 8009ae1 ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/DestroyPrincipalsServerAction.java 93daae8 ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/FinalizeKerberosServerAction.java c710b8e ambari-server/src/main/java/org/apache/ambari/server/utils/RequestUtils.java PRE-CREATION ambari-server/src/main/resources/webapp/WEB-INF/spring-security.xml 3bbc785 ambari-server/src/test/java/org/apache/ambari/server/actionmanager/TestActionDBAccessorImpl.java f88cf8e ambari-server/src/test/java/org/apache/ambari/server/actionmanager/TestActionManager.java d7d08b1 ambari-server/src/test/java/org/apache/ambari/server/agent/HeartbeatProcessorTest.java 7f3d763 ambari-server/src/test/java/org/apache/ambari/server/api/services/BaseServiceTest.java 19eeffb ambari-server/src/test/java/org/apache/ambari/server/audit/AccessUnauthorizedAuditEventTest.java PRE-CREATION ambari-server/src/test/java/org/apache/ambari/server/audit/LoginAuditEventTest.java PRE-CREATION ambari-server/src/test/java/org/apache/ambari/server/audit/LogoutAuditEventTest.java PRE-CREATION ambari-server/src/test/java/org/apache/ambari/server/audit/OperationStatusAuditEventTest.java PRE-CREATION ambari-server/src/test/java/org/apache/ambari/server/audit/StartOperationRequestAuditEventTest.java PRE-CREATION ambari-server/src/test/java/org/apache/ambari/server/audit/request/AbstractBaseCreator.java PRE-CREATION ambari-server/src/test/java/org/apache/ambari/server/audit/request/AllGetCreator.java PRE-CREATION ambari-server/src/test/java/org/apache/ambari/server/audit/request/AllPostAndPutCreator.java PRE-CREATION ambari-server/src/test/java/org/apache/ambari/server/audit/request/DefaultEventCreatorTest.java PRE-CREATION ambari-server/src/test/java/org/apache/ambari/server/audit/request/PutHostComponentCreator.java PRE-CREATION ambari-server/src/test/java/org/apache/ambari/server/audit/request/RequestAuditLogModule.java PRE-CREATION ambari-server/src/test/java/org/apache/ambari/server/audit/request/RequestAuditLoggerTest.java PRE-CREATION ambari-server/src/test/java/org/apache/ambari/server/checks/UpgradeCheckOrderTest.java d4ff566 ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerTest.java ca062c0 ambari-server/src/test/java/org/apache/ambari/server/controller/KerberosHelperTest.java f6027f3 ambari-server/src/test/java/org/apache/ambari/server/notifications/DispatchFactoryTest.java 6834d5c ambari-server/src/test/java/org/apache/ambari/server/orm/InMemoryDefaultTestModule.java 3ecfe14 ambari-server/src/test/java/org/apache/ambari/server/orm/JdbcPropertyTest.java c07382b ambari-server/src/test/java/org/apache/ambari/server/security/authentication/AmbariAuthenticationFilterTest.java PRE-CREATION ambari-server/src/test/java/org/apache/ambari/server/security/authorization/AmbariAuthorizationFilterTest.java 9db3904 ambari-server/src/test/java/org/apache/ambari/server/security/authorization/AmbariLdapAuthenticationProviderForDNWithSpaceTest.java da8d9bc ambari-server/src/test/java/org/apache/ambari/server/security/authorization/AmbariLdapAuthenticationProviderTest.java d48be85 ambari-server/src/test/java/org/apache/ambari/server/security/authorization/AmbariLocalUserDetailsServiceTest.java c410f5b ambari-server/src/test/java/org/apache/ambari/server/security/authorization/LdapServerPropertiesTest.java 0797239 ambari-server/src/test/java/org/apache/ambari/server/serveraction/kerberos/KerberosServerActionTest.java ce97f25 ambari-server/src/test/java/org/apache/ambari/server/serveraction/kerberos/UpdateKerberosConfigsServerActionTest.java d6f9efe ambari-server/src/test/java/org/apache/ambari/server/utils/RequestUtilsTest.java PRE-CREATION Diff: https://reviews.apache.org/r/44265/diff/ Testing --- Unit tests are added to cover functionality. All tests passed on local machine. Thanks, Daniel Gergely
Re: Review Request 43832: AMBARI-14627: Ability to automate setup-security and setup-ldap/sync-ldap
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/43832/#review124541 --- ambari-server/src/main/python/ambari_server/userInput.py (lines 99 - 102) <https://reviews.apache.org/r/43832/#comment187155> As I see when validator fails for a parameter that value is set from an argument, it is asked interactively again. Is this intentional? When using command line parameters I would expect non-interactive behaviour. (e.g. the command is assembled and run by a script) What do you think of terminating execution with an exit code when validation fails for a value that is set as an argument. ambari-server/src/main/python/ambari_server/userInput.py (lines 111 - 114) <https://reviews.apache.org/r/43832/#comment187156> See my comment above - Daniel Gergely On márc. 16, 2016, 5:14 du, Oliver Szabo wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/43832/ > --- > > (Updated márc. 16, 2016, 5:14 du) > > > Review request for Ambari, Alejandro Fernandez, Andrew Onischuk, Robert > Levas, Sumit Mohanty, and Sebastian Toader. > > > Bugs: AMBARI-14627 > https://issues.apache.org/jira/browse/AMBARI-14627 > > > Repository: ambari > > > Description > --- > > Added ability to automate setup-security/setup-ldap and sync-ldap. Ambari > uses '--' flags in order to replace user inputs. (if one of the flag is > missing, ambari will ask for user input) > Example usage: > > 1.) LDAP setup: > ambari-server setup-ldap \ > --ldap-url="ldap.hortonworks.com:389" \ > --ldap-secondary-url="" \ > --ldap-ssl="false" \ > --ldap-user-class="person" \ > --ldap-user-attr="sAMAccountName" \ > --ldap-group-class="group" \ > --ldap-group-attr="cn" \ > --ldap-member-attr="member" \ > --ldap-dn="distunguishedName" \ > --ldap-base-dn="dc=hdp01,dc=local" \ > --ldap-referral="" \ > --ldap-bind-anonym=false \ > --ldap-manager-dn="cn=hdfs,ou=hdp,dc=hdp01,dc=local" \ > --ldap-manager-password="myldappassword" \ > --ldap-save-settings \ > --truststore-type="jks" \ > --truststore-path="/var/lib/ambari-server/keys/jkskeystore.jks" \ > --truststore-password="mypass" > > 2.) Ldap sync: > ambari-server sync-ldap --groups=groups.txt --ldap-sync-admin-name=admin > --ldap-sync-admin-password=admin > > 3.) Setup Https: > ambari-server setup-security \ > --security-option=setup-https \ > --security-keys_dir=/var/lib/ambari-server/keys \ > --api-ssl=true --client-api-ssl-port=8443 \ > --import-cert-path=/var/lib/ambari-server/keys/my.crt \ > --import-key-path=/var/lib/ambari-server/keys/my.key \ > --pem-password=password > 4.) Encrypt passwords: > ambari-server setup-security --security-option=encrypt-password > --master-key=masterkey --master-key-persist=true > > 5.) Setup Kerberos JAAS: > ambari-server setup-security --security-option=setup-kerberos-jaas > --jaas-principal="amb...@example.com" > --jaas-keytab="/etc/security/keytabs/ambari.keytab" > > 6.) Setup TrustStore: > ambari-server setup-security \ > --security-option=setup-truststore \ > --truststore-path=/var/lib/ambari-server/keys/keystore.p12 \ > --truststore-type=pkcs12 \ > --truststore-password=password \ > --truststore-reconfigure // not needed if not configured - also, this > option is not available on branch-2.2 > 7.) Import certificate to TrustStore: > ambari-server setup-security \ > --security-option=import-certificate \ > --truststore-path=/var/lib/ambari-server/keys/keystore.p12 \ > --truststore-type=pkcs12 \ > --truststore-password=password \ > --import-cert-path=/var/lib/ambari-server/oleewere.crt \ > --import-cert-alias=myalias \ > --truststore-reconfigure // not needed if not configured - also, this > option is not available on branch-2.2 > > > Diffs > - > > ambari-server/src/main/python/ambari-server.py bc86d32 > ambari-server/src/main/python/ambari_server/dbConfiguration.py 5519a3d > ambari-server/src/main/python/ambari_server/dbConfiguration_linux.py > 59c5d85 > ambari-server/src/main/python/ambari_server/dbConfiguration_windows.py >
Re: Review Request 44958: Increase Ambari Server Perm gen default value
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44958/ --- (Updated márc. 18, 2016, 8:39 de) Review request for Ambari, Laszlo Puskas, Oliver Szabo, Robert Levas, Sandor Magyari, and Sebastian Toader. Changes --- Removing -XX:+CMSPermGenSweepingEnabled, since it is not used from oracle java 7. Bugs: AMBARI-15465 https://issues.apache.org/jira/browse/AMBARI-15465 Repository: ambari Description --- Increased permGen size to 128m, also added **-XX:+CMSPermGenSweepingEnabled** and **-XX:+CMSClassUnloadingEnabled** to allow jvm to keep permGen space clean. Diffs (updated) - ambari-server/conf/unix/ambari-env.sh e9fdcdd ambari-server/src/main/python/ambari_server_main.py 00e084b Diff: https://reviews.apache.org/r/44958/diff/ Testing --- Manual testing: [root@c6401 sbin]# ps -ef | grep ambari-server root 20769 1 27 14:47 pts/000:00:24 //usr/jdk64/jdk1.8.0_60/bin/java -server -XX:NewRatio=3 -XX:+UseConcMarkSweepGC -XX:-UseGCOverheadLimit -XX:CMSInitiatingOccupancyFraction=60 **-XX:+CMSPermGenSweepingEnabled -XX:+CMSClassUnloadingEnabled** -Dsun.zip.disableMemoryMapping=true -Xms512m -Xmx2048m **-XX:MaxPermSize=128m** -Djava.security.auth.login.config=//etc/ambari-server/conf/krb5JAASLogin.conf -Djava.security.krb5.conf=/etc/krb5.conf -Djavax.security.auth.useSubjectCredsOnly=false -Xms512m -Xmx2048m -Djava.security.auth.login.config=//etc/ambari-server/conf/krb5JAASLogin.conf -Djava.security.krb5.conf=/etc/krb5.conf -Djavax.security.auth.useSubjectCredsOnly=false -cp //etc/ambari-server/conf:/usr/lib/ambari-server/*:/usr/share/java/postgresql-jdbc.jar org.apache.ambari.server.controller.AmbariServer Thanks, Daniel Gergely
Re: Review Request 44958: Increase Ambari Server Perm gen default value
> On márc. 17, 2016, 4:37 du, Robert Levas wrote: > > ambari-server/src/main/python/ambari_server_main.py, line 89 > > <https://reviews.apache.org/r/44958/diff/1/?file=1302356#file1302356line89> > > > > I am not familiar with these options, but it apears like no one really > > knows what they do. See https://dzone.com/articles/busting-permgen-myths > > for an example. > > > > Do we really think that this is necessary? Do we know how this will > > affect performance? These options are for JVM, it can be checked in the jvm implementors webpage. For example IBM does not use permgen space at all, but oracle does. The first option has not effect from java 7, but it was useful in java 6 and before, so I removed that one. However -XX:+CMSClassUnloadingEnabled is useful. When classes are loaded (statically or dynamically) they are stored in permgen space and remains there forever (well, almost...). Since we use CMS (-XX:+UseConcMarkSweepGC flag is already defined), we can allow GC to unload unused classes. As a result it can increase the free space in permGen. In normal case when permGen space is full, garbage collector initiates a STW (stop-the-world) action to clean permgen space. Short description here: https://blogs.oracle.com/poonam/entry/about_g1_garbage_collector_permanent Oracle tutorial here: http://www.oracle.com/technetwork/tutorials/tutorials-1876574.html >From Java 8 as I know there is no permGen space, so this issue relates only to >java 7. So I removed -XX:+CMSPermGenSweepingEnabled, but I kept -XX:+CMSClassUnloadingEnabled. Performance wise you should not experience any difference. - Daniel --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44958/#review124042 ------- On márc. 17, 2016, 2:57 du, Daniel Gergely wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/44958/ > --- > > (Updated márc. 17, 2016, 2:57 du) > > > Review request for Ambari, Laszlo Puskas, Oliver Szabo, Robert Levas, Sandor > Magyari, and Sebastian Toader. > > > Bugs: AMBARI-15465 > https://issues.apache.org/jira/browse/AMBARI-15465 > > > Repository: ambari > > > Description > --- > > Increased permGen size to 128m, also added **-XX:+CMSPermGenSweepingEnabled** > and **-XX:+CMSClassUnloadingEnabled** to allow jvm to keep permGen space > clean. > > > Diffs > - > > ambari-server/conf/unix/ambari-env.sh e9fdcdd > ambari-server/src/main/python/ambari_server_main.py 00e084b > > Diff: https://reviews.apache.org/r/44958/diff/ > > > Testing > --- > > Manual testing: > [root@c6401 sbin]# ps -ef | grep ambari-server > root 20769 1 27 14:47 pts/000:00:24 > //usr/jdk64/jdk1.8.0_60/bin/java -server -XX:NewRatio=3 > -XX:+UseConcMarkSweepGC -XX:-UseGCOverheadLimit > -XX:CMSInitiatingOccupancyFraction=60 **-XX:+CMSPermGenSweepingEnabled > -XX:+CMSClassUnloadingEnabled** -Dsun.zip.disableMemoryMapping=true -Xms512m > -Xmx2048m **-XX:MaxPermSize=128m** > -Djava.security.auth.login.config=//etc/ambari-server/conf/krb5JAASLogin.conf > -Djava.security.krb5.conf=/etc/krb5.conf > -Djavax.security.auth.useSubjectCredsOnly=false -Xms512m -Xmx2048m > -Djava.security.auth.login.config=//etc/ambari-server/conf/krb5JAASLogin.conf > -Djava.security.krb5.conf=/etc/krb5.conf > -Djavax.security.auth.useSubjectCredsOnly=false -cp > //etc/ambari-server/conf:/usr/lib/ambari-server/*:/usr/share/java/postgresql-jdbc.jar > org.apache.ambari.server.controller.AmbariServer > > > Thanks, > > Daniel Gergely > >
Review Request 44956: Topology host info is not cleared when a host is removed
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44956/ --- Review request for Ambari, Alejandro Fernandez, Laszlo Puskas, Oliver Szabo, Sandor Magyari, Sumit Mohanty, and Sebastian Toader. Bugs: AMBARI-15457 https://issues.apache.org/jira/browse/AMBARI-15457 Repository: ambari Description --- When a host is removed, a record is left in the topology_host_info table. As a result re-adding the same host is not possible, due to invalid topology. Host removal should remove the corresponding record from topology_host_info as well. A new column with a foreign key to hosts table is added. In addition ClusterTopologyImpl was statefull and hostGroupInfoMap was not updated when a host was removed. Diffs - ambari-server/src/main/java/org/apache/ambari/server/controller/internal/HostResourceProvider.java 6101ee8 ambari-server/src/main/java/org/apache/ambari/server/orm/dao/TopologyHostInfoDAO.java PRE-CREATION ambari-server/src/main/java/org/apache/ambari/server/orm/entities/TopologyHostInfoEntity.java a4f251a ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ClustersImpl.java 46dd49d ambari-server/src/main/java/org/apache/ambari/server/topology/ClusterTopology.java 4e178c0 ambari-server/src/main/java/org/apache/ambari/server/topology/ClusterTopologyImpl.java 003539c ambari-server/src/main/java/org/apache/ambari/server/topology/HostGroupInfo.java 4847a11 ambari-server/src/main/java/org/apache/ambari/server/topology/PersistedState.java dbf6735 ambari-server/src/main/java/org/apache/ambari/server/topology/PersistedStateImpl.java b878955 ambari-server/src/main/java/org/apache/ambari/server/topology/TopologyManager.java 41f3150 ambari-server/src/main/java/org/apache/ambari/server/upgrade/UpgradeCatalog222.java 8267d5d ambari-server/src/main/resources/Ambari-DDL-MySQL-CREATE.sql dee397f ambari-server/src/main/resources/Ambari-DDL-Oracle-CREATE.sql 253b5d7 ambari-server/src/main/resources/Ambari-DDL-Postgres-CREATE.sql 832d1bc ambari-server/src/main/resources/Ambari-DDL-Postgres-EMBEDDED-CREATE.sql 6c8a09d ambari-server/src/main/resources/Ambari-DDL-SQLAnywhere-CREATE.sql a6acb74 ambari-server/src/main/resources/Ambari-DDL-SQLServer-CREATE.sql 2957c5c ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerTest.java 3e43ef1 ambari-server/src/test/java/org/apache/ambari/server/controller/internal/HostResourceProviderTest.java 69c570e Diff: https://reviews.apache.org/r/44956/diff/ Testing --- Tests ran without failures. (2016-03-17 13:30) Thanks, Daniel Gergely
Re: Review Request 44956: Topology host info is not cleared when a host is removed
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44956/ --- (Updated márc. 18, 2016, 9:27 de) Review request for Ambari, Alejandro Fernandez, Laszlo Puskas, Oliver Szabo, Sandor Magyari, Sumit Mohanty, and Sebastian Toader. Bugs: AMBARI-15457 https://issues.apache.org/jira/browse/AMBARI-15457 Repository: ambari Description --- When a host is removed, a record is left in the topology_host_info table. As a result re-adding the same host is not possible, due to invalid topology. Host removal should remove the corresponding record from topology_host_info as well. A new column with a foreign key to hosts table is added. In addition ClusterTopologyImpl was statefull and hostGroupInfoMap was not updated when a host was removed. Diffs (updated) - ambari-server/src/main/java/org/apache/ambari/server/controller/internal/HostResourceProvider.java 6101ee8 ambari-server/src/main/java/org/apache/ambari/server/orm/dao/TopologyHostInfoDAO.java PRE-CREATION ambari-server/src/main/java/org/apache/ambari/server/orm/entities/TopologyHostInfoEntity.java a4f251a ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ClustersImpl.java 46dd49d ambari-server/src/main/java/org/apache/ambari/server/topology/ClusterTopology.java 4e178c0 ambari-server/src/main/java/org/apache/ambari/server/topology/ClusterTopologyImpl.java 003539c ambari-server/src/main/java/org/apache/ambari/server/topology/HostGroupInfo.java 4847a11 ambari-server/src/main/java/org/apache/ambari/server/topology/PersistedState.java dbf6735 ambari-server/src/main/java/org/apache/ambari/server/topology/PersistedStateImpl.java b878955 ambari-server/src/main/java/org/apache/ambari/server/topology/TopologyManager.java 41f3150 ambari-server/src/main/java/org/apache/ambari/server/upgrade/UpgradeCatalog222.java 8267d5d ambari-server/src/main/resources/Ambari-DDL-MySQL-CREATE.sql dee397f ambari-server/src/main/resources/Ambari-DDL-Oracle-CREATE.sql 253b5d7 ambari-server/src/main/resources/Ambari-DDL-Postgres-CREATE.sql 832d1bc ambari-server/src/main/resources/Ambari-DDL-Postgres-EMBEDDED-CREATE.sql 6c8a09d ambari-server/src/main/resources/Ambari-DDL-SQLAnywhere-CREATE.sql a6acb74 ambari-server/src/main/resources/Ambari-DDL-SQLServer-CREATE.sql 2957c5c ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerTest.java 3e43ef1 ambari-server/src/test/java/org/apache/ambari/server/controller/internal/HostResourceProviderTest.java 69c570e Diff: https://reviews.apache.org/r/44956/diff/ Testing --- Tests ran without failures. (2016-03-17 13:30) Thanks, Daniel Gergely
Re: Review Request 44509: Blueprints: NullPointerException when unncessary config types found with %HOSTGROUP% tags
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44509/#review122515 --- Ship it! Ship It! - Daniel Gergely On márc. 8, 2016, 1:42 du, Sebastian Toader wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/44509/ > --- > > (Updated márc. 8, 2016, 1:42 du) > > > Review request for Ambari, Daniel Gergely, Oliver Szabo, and Sandor Magyari. > > > Bugs: AMBARI-15336 > https://issues.apache.org/jira/browse/AMBARI-15336 > > > Repository: ambari > > > Description > --- > > This issue was originally addressed by > https://issues.apache.org/jira/browse/AMBARI-14745 however a corner case is > not being handled in that fix. > > If a Blueprint is being used where configurations are specified solely within > host groups than the orphaned configuration types were missed from removal. > > > Diffs > - > > > ambari-server/src/main/java/org/apache/ambari/server/topology/ClusterConfigurationRequest.java > a995a3b > > ambari-server/src/test/java/org/apache/ambari/server/topology/ClusterConfigurationRequestTest.java > 3ed8ed5 > > ambari-server/src/test/java/org/apache/ambari/server/topology/ClusterInstallWithoutStartTest.java > 156580a > > ambari-server/src/test/java/org/apache/ambari/server/topology/TopologyManagerTest.java > 69c1935 > > Diff: https://reviews.apache.org/r/44509/diff/ > > > Testing > --- > > 1.Unitest have been modified to cover this corner case as well. > > 2. Manual testing with Blueprints having configurations only in host groups. > > > Thanks, > > Sebastian Toader > >
