You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ambari.apache.org by Dmytro Shkvyra <ds...@hortonworks.com> on 2014/01/31 19:39:31 UTC

Review Request 17596: If Ganglia is not installed, server logs hundreds of error messages

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17596/
-----------------------------------------------------------

Review request for Ambari, Mahadev Konar and Yusaku Sako.


Bugs: AMBARI-4490
    https://issues.apache.org/jira/browse/AMBARI-4490


Repository: ambari


Description
-------


Diffs
-----

  ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ResourceImpl.java 15fb961 
  ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ResourceImplTest.java da87bc6 

Diff: https://reviews.apache.org/r/17596/diff/


Testing
-------

Added unitests


Thanks,

Dmytro Shkvyra


Re: Review Request 17596: If Ganglia is not installed, server logs hundreds of error messages

Posted by Dmytro Shkvyra <ds...@hortonworks.com>.

> On Feb. 1, 2014, 12:37 a.m., Nate Cole wrote:
> > This patch doesn't address the real issue - that being the port lookup in AbstractProviderModule is not returning (or finding) a value for ambari.dfs.datanode.http.port.  The error propagates itself in JMXPropertyProvider when the URL "spec" is being built.

Frankly, I have tested it on HDP-1.3.3 and error messages have dissapeared. Please see Yusako's comments in https://hortonworks.jira.com/browse/BUG-12238
I have proposed more common aproach for solve this issue. If we use only "spec" for resolve port, we should copypaste code to all places when we can have similar properties. Also, it will be more complicated than common solution for support of changes.


- Dmytro


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17596/#review33374
-----------------------------------------------------------


On Jan. 31, 2014, 9:47 p.m., Dmytro Shkvyra wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17596/
> -----------------------------------------------------------
> 
> (Updated Jan. 31, 2014, 9:47 p.m.)
> 
> 
> Review request for Ambari, Mahadev Konar, Nate Cole, Tom Beerbower, and Yusaku Sako.
> 
> 
> Bugs: AMBARI-4490
>     https://issues.apache.org/jira/browse/AMBARI-4490
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> The cause of error messages is that we start use new format properties like "dfs.datanode.http.address":"0.0.0.0:${ambari.dfs.datanode.http.port}".
> It means that we have to replace ${ambari.dfs.datanode.http.port} with value of ambari.dfs.datanode.http.port property from current config.
> In this case we need process these value references and keep in mind that:
> 1) We can have some references in one property, like this "dfs.datanode.http.address":"${ambari.dfs.datanode.http.host}:${ambari.dfs.datanode.http.port}"
> 2) Also value references can be referenced to another references.
> 3) Patch have to impact all configs
> So, I have created private method postProcessPropertyValue in ResourceImpl.java for resolve all of value references
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ResourceImpl.java 15fb961 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ResourceImplTest.java da87bc6 
> 
> Diff: https://reviews.apache.org/r/17596/diff/
> 
> 
> Testing
> -------
> 
> Added unitests
> 
> 
> Thanks,
> 
> Dmytro Shkvyra
> 
>


Re: Review Request 17596: If Ganglia is not installed, server logs hundreds of error messages

Posted by Nate Cole <nc...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17596/#review33374
-----------------------------------------------------------


This patch doesn't address the real issue - that being the port lookup in AbstractProviderModule is not returning (or finding) a value for ambari.dfs.datanode.http.port.  The error propagates itself in JMXPropertyProvider when the URL "spec" is being built.

- Nate Cole


On Jan. 31, 2014, 4:47 p.m., Dmytro Shkvyra wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17596/
> -----------------------------------------------------------
> 
> (Updated Jan. 31, 2014, 4:47 p.m.)
> 
> 
> Review request for Ambari, Mahadev Konar, Nate Cole, Tom Beerbower, and Yusaku Sako.
> 
> 
> Bugs: AMBARI-4490
>     https://issues.apache.org/jira/browse/AMBARI-4490
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> The cause of error messages is that we start use new format properties like "dfs.datanode.http.address":"0.0.0.0:${ambari.dfs.datanode.http.port}".
> It means that we have to replace ${ambari.dfs.datanode.http.port} with value of ambari.dfs.datanode.http.port property from current config.
> In this case we need process these value references and keep in mind that:
> 1) We can have some references in one property, like this "dfs.datanode.http.address":"${ambari.dfs.datanode.http.host}:${ambari.dfs.datanode.http.port}"
> 2) Also value references can be referenced to another references.
> 3) Patch have to impact all configs
> So, I have created private method postProcessPropertyValue in ResourceImpl.java for resolve all of value references
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ResourceImpl.java 15fb961 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ResourceImplTest.java da87bc6 
> 
> Diff: https://reviews.apache.org/r/17596/diff/
> 
> 
> Testing
> -------
> 
> Added unitests
> 
> 
> Thanks,
> 
> Dmytro Shkvyra
> 
>


Re: Review Request 17596: If Ganglia is not installed, server logs hundreds of error messages

Posted by Nate Cole <nc...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17596/#review33810
-----------------------------------------------------------

Ship it!


Ship It!

- Nate Cole


On Feb. 5, 2014, 11:54 a.m., Dmytro Shkvyra wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17596/
> -----------------------------------------------------------
> 
> (Updated Feb. 5, 2014, 11:54 a.m.)
> 
> 
> Review request for Ambari, Mahadev Konar, Nate Cole, Tom Beerbower, and Yusaku Sako.
> 
> 
> Bugs: AMBARI-4490
>     https://issues.apache.org/jira/browse/AMBARI-4490
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> The cause of error messages is that we start use new format properties like "dfs.datanode.http.address":"0.0.0.0:${ambari.dfs.datanode.http.port}".
> It means that we have to replace ${ambari.dfs.datanode.http.port} with value of ambari.dfs.datanode.http.port property from current config.
> In this case we need process these value references and keep in mind that:
> 1) We can have some references in one property, like this "dfs.datanode.http.address":"${ambari.dfs.datanode.http.host}:${ambari.dfs.datanode.http.port}"
> 2) Also value references can be referenced to another references.
> 3) Patch have to impact all configs
> So, I have created private method postProcessPropertyValue in ResourceImpl.java for resolve all of value references
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AbstractProviderModule.java f39893c 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/internal/JMXHostProviderTest.java 1a1bad6 
> 
> Diff: https://reviews.apache.org/r/17596/diff/
> 
> 
> Testing
> -------
> 
> Added unitests
> 
> 
> Thanks,
> 
> Dmytro Shkvyra
> 
>


Re: Review Request 17596: If Ganglia is not installed, server logs hundreds of error messages

Posted by Tom Beerbower <tb...@hortonworks.com>.

> On Feb. 10, 2014, 6:26 p.m., Tom Beerbower wrote:
> > Minor :
> > 
> > Would it be better to use ...
> > 
> >     if (object instanceof String) {
> > 
> > instead of ... 
> > 
> >     if (object != null && object.getClass().equals(String.class)){
> > 
> > ?
> 
> Nate Cole wrote:
>     I agree with Tom - I've also used String.class.isInstance(object)

My personal preference is 'object instanceof String' simply because it reads more like how you would describe the line in English, which makes the code much easier to read.  Sort of like how 'myString.equals("FOO")' is much easier to read than '"FOO".equals(myString)' (which drives me nuts).


- Tom


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17596/#review34089
-----------------------------------------------------------


On Feb. 5, 2014, 4:54 p.m., Dmytro Shkvyra wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17596/
> -----------------------------------------------------------
> 
> (Updated Feb. 5, 2014, 4:54 p.m.)
> 
> 
> Review request for Ambari, Mahadev Konar, Nate Cole, Tom Beerbower, and Yusaku Sako.
> 
> 
> Bugs: AMBARI-4490
>     https://issues.apache.org/jira/browse/AMBARI-4490
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> The cause of error messages is that we start use new format properties like "dfs.datanode.http.address":"0.0.0.0:${ambari.dfs.datanode.http.port}".
> It means that we have to replace ${ambari.dfs.datanode.http.port} with value of ambari.dfs.datanode.http.port property from current config.
> In this case we need process these value references and keep in mind that:
> 1) We can have some references in one property, like this "dfs.datanode.http.address":"${ambari.dfs.datanode.http.host}:${ambari.dfs.datanode.http.port}"
> 2) Also value references can be referenced to another references.
> 3) Patch have to impact all configs
> So, I have created private method postProcessPropertyValue in ResourceImpl.java for resolve all of value references
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AbstractProviderModule.java f39893c 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/internal/JMXHostProviderTest.java 1a1bad6 
> 
> Diff: https://reviews.apache.org/r/17596/diff/
> 
> 
> Testing
> -------
> 
> Added unitests
> 
> 
> Thanks,
> 
> Dmytro Shkvyra
> 
>


Re: Review Request 17596: If Ganglia is not installed, server logs hundreds of error messages

Posted by Nate Cole <nc...@hortonworks.com>.

> On Feb. 10, 2014, 1:26 p.m., Tom Beerbower wrote:
> > Minor :
> > 
> > Would it be better to use ...
> > 
> >     if (object instanceof String) {
> > 
> > instead of ... 
> > 
> >     if (object != null && object.getClass().equals(String.class)){
> > 
> > ?

I agree with Tom - I've also used String.class.isInstance(object)


- Nate


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17596/#review34089
-----------------------------------------------------------


On Feb. 5, 2014, 11:54 a.m., Dmytro Shkvyra wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17596/
> -----------------------------------------------------------
> 
> (Updated Feb. 5, 2014, 11:54 a.m.)
> 
> 
> Review request for Ambari, Mahadev Konar, Nate Cole, Tom Beerbower, and Yusaku Sako.
> 
> 
> Bugs: AMBARI-4490
>     https://issues.apache.org/jira/browse/AMBARI-4490
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> The cause of error messages is that we start use new format properties like "dfs.datanode.http.address":"0.0.0.0:${ambari.dfs.datanode.http.port}".
> It means that we have to replace ${ambari.dfs.datanode.http.port} with value of ambari.dfs.datanode.http.port property from current config.
> In this case we need process these value references and keep in mind that:
> 1) We can have some references in one property, like this "dfs.datanode.http.address":"${ambari.dfs.datanode.http.host}:${ambari.dfs.datanode.http.port}"
> 2) Also value references can be referenced to another references.
> 3) Patch have to impact all configs
> So, I have created private method postProcessPropertyValue in ResourceImpl.java for resolve all of value references
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AbstractProviderModule.java f39893c 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/internal/JMXHostProviderTest.java 1a1bad6 
> 
> Diff: https://reviews.apache.org/r/17596/diff/
> 
> 
> Testing
> -------
> 
> Added unitests
> 
> 
> Thanks,
> 
> Dmytro Shkvyra
> 
>


Re: Review Request 17596: If Ganglia is not installed, server logs hundreds of error messages

Posted by Tom Beerbower <tb...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17596/#review34089
-----------------------------------------------------------


Minor :

Would it be better to use ...

    if (object instanceof String) {

instead of ... 

    if (object != null && object.getClass().equals(String.class)){

?

- Tom Beerbower


On Feb. 5, 2014, 4:54 p.m., Dmytro Shkvyra wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17596/
> -----------------------------------------------------------
> 
> (Updated Feb. 5, 2014, 4:54 p.m.)
> 
> 
> Review request for Ambari, Mahadev Konar, Nate Cole, Tom Beerbower, and Yusaku Sako.
> 
> 
> Bugs: AMBARI-4490
>     https://issues.apache.org/jira/browse/AMBARI-4490
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> The cause of error messages is that we start use new format properties like "dfs.datanode.http.address":"0.0.0.0:${ambari.dfs.datanode.http.port}".
> It means that we have to replace ${ambari.dfs.datanode.http.port} with value of ambari.dfs.datanode.http.port property from current config.
> In this case we need process these value references and keep in mind that:
> 1) We can have some references in one property, like this "dfs.datanode.http.address":"${ambari.dfs.datanode.http.host}:${ambari.dfs.datanode.http.port}"
> 2) Also value references can be referenced to another references.
> 3) Patch have to impact all configs
> So, I have created private method postProcessPropertyValue in ResourceImpl.java for resolve all of value references
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AbstractProviderModule.java f39893c 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/internal/JMXHostProviderTest.java 1a1bad6 
> 
> Diff: https://reviews.apache.org/r/17596/diff/
> 
> 
> Testing
> -------
> 
> Added unitests
> 
> 
> Thanks,
> 
> Dmytro Shkvyra
> 
>


Re: Review Request 17596: If Ganglia is not installed, server logs hundreds of error messages

Posted by Tom Beerbower <tb...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17596/#review34175
-----------------------------------------------------------

Ship it!


In this line ...

    if (object != null && object instanceof String){

It doesn't hurt anything but the null check is not needed.  'object instanceof String' will evaluate to false if object is null.


- Tom Beerbower


On Feb. 11, 2014, 11:15 a.m., Dmytro Shkvyra wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17596/
> -----------------------------------------------------------
> 
> (Updated Feb. 11, 2014, 11:15 a.m.)
> 
> 
> Review request for Ambari, Mahadev Konar, Nate Cole, Tom Beerbower, and Yusaku Sako.
> 
> 
> Bugs: AMBARI-4490
>     https://issues.apache.org/jira/browse/AMBARI-4490
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> The cause of error messages is that we start use new format properties like "dfs.datanode.http.address":"0.0.0.0:${ambari.dfs.datanode.http.port}".
> It means that we have to replace ${ambari.dfs.datanode.http.port} with value of ambari.dfs.datanode.http.port property from current config.
> In this case we need process these value references and keep in mind that:
> 1) We can have some references in one property, like this "dfs.datanode.http.address":"${ambari.dfs.datanode.http.host}:${ambari.dfs.datanode.http.port}"
> 2) Also value references can be referenced to another references.
> 3) Patch have to impact all configs
> So, I have created private method postProcessPropertyValue in ResourceImpl.java for resolve all of value references
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AbstractProviderModule.java f39893c 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/internal/JMXHostProviderTest.java 1a1bad6 
> 
> Diff: https://reviews.apache.org/r/17596/diff/
> 
> 
> Testing
> -------
> 
> Added unitests
> 
> 
> Thanks,
> 
> Dmytro Shkvyra
> 
>


Re: Review Request 17596: If Ganglia is not installed, server logs hundreds of error messages

Posted by Dmytro Shkvyra <ds...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17596/
-----------------------------------------------------------

(Updated Feb. 11, 2014, 11:15 a.m.)


Review request for Ambari, Mahadev Konar, Nate Cole, Tom Beerbower, and Yusaku Sako.


Changes
-------

Replced "object.getClass().equals(String.class)" with "object instanceof String"


Bugs: AMBARI-4490
    https://issues.apache.org/jira/browse/AMBARI-4490


Repository: ambari


Description
-------

The cause of error messages is that we start use new format properties like "dfs.datanode.http.address":"0.0.0.0:${ambari.dfs.datanode.http.port}".
It means that we have to replace ${ambari.dfs.datanode.http.port} with value of ambari.dfs.datanode.http.port property from current config.
In this case we need process these value references and keep in mind that:
1) We can have some references in one property, like this "dfs.datanode.http.address":"${ambari.dfs.datanode.http.host}:${ambari.dfs.datanode.http.port}"
2) Also value references can be referenced to another references.
3) Patch have to impact all configs
So, I have created private method postProcessPropertyValue in ResourceImpl.java for resolve all of value references


Diffs (updated)
-----

  ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AbstractProviderModule.java f39893c 
  ambari-server/src/test/java/org/apache/ambari/server/controller/internal/JMXHostProviderTest.java 1a1bad6 

Diff: https://reviews.apache.org/r/17596/diff/


Testing
-------

Added unitests


Thanks,

Dmytro Shkvyra


Re: Review Request 17596: If Ganglia is not installed, server logs hundreds of error messages

Posted by Dmytro Shkvyra <ds...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17596/
-----------------------------------------------------------

(Updated Feb. 5, 2014, 4:54 p.m.)


Review request for Ambari, Mahadev Konar, Nate Cole, Tom Beerbower, and Yusaku Sako.


Changes
-------

All Nate Cole's requirements were implemented.


Bugs: AMBARI-4490
    https://issues.apache.org/jira/browse/AMBARI-4490


Repository: ambari


Description
-------

The cause of error messages is that we start use new format properties like "dfs.datanode.http.address":"0.0.0.0:${ambari.dfs.datanode.http.port}".
It means that we have to replace ${ambari.dfs.datanode.http.port} with value of ambari.dfs.datanode.http.port property from current config.
In this case we need process these value references and keep in mind that:
1) We can have some references in one property, like this "dfs.datanode.http.address":"${ambari.dfs.datanode.http.host}:${ambari.dfs.datanode.http.port}"
2) Also value references can be referenced to another references.
3) Patch have to impact all configs
So, I have created private method postProcessPropertyValue in ResourceImpl.java for resolve all of value references


Diffs (updated)
-----

  ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AbstractProviderModule.java f39893c 
  ambari-server/src/test/java/org/apache/ambari/server/controller/internal/JMXHostProviderTest.java 1a1bad6 

Diff: https://reviews.apache.org/r/17596/diff/


Testing
-------

Added unitests


Thanks,

Dmytro Shkvyra


Re: Review Request 17596: If Ganglia is not installed, server logs hundreds of error messages

Posted by Nate Cole <nc...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17596/#review33640
-----------------------------------------------------------


I just consulted with Yusaku on this issue.  Please see the JIRA issue for clarification.

- Nate Cole


On Feb. 4, 2014, 3:44 p.m., Dmytro Shkvyra wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17596/
> -----------------------------------------------------------
> 
> (Updated Feb. 4, 2014, 3:44 p.m.)
> 
> 
> Review request for Ambari, Mahadev Konar, Nate Cole, Tom Beerbower, and Yusaku Sako.
> 
> 
> Bugs: AMBARI-4490
>     https://issues.apache.org/jira/browse/AMBARI-4490
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> The cause of error messages is that we start use new format properties like "dfs.datanode.http.address":"0.0.0.0:${ambari.dfs.datanode.http.port}".
> It means that we have to replace ${ambari.dfs.datanode.http.port} with value of ambari.dfs.datanode.http.port property from current config.
> In this case we need process these value references and keep in mind that:
> 1) We can have some references in one property, like this "dfs.datanode.http.address":"${ambari.dfs.datanode.http.host}:${ambari.dfs.datanode.http.port}"
> 2) Also value references can be referenced to another references.
> 3) Patch have to impact all configs
> So, I have created private method postProcessPropertyValue in ResourceImpl.java for resolve all of value references
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AbstractProviderModule.java f39893c 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ConfigurationResourceProvider.java 60a3780 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/EvaluatedResourceImpl.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ResourceImpl.java 15fb961 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/internal/EvaluatedResourceImplTest.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/17596/diff/
> 
> 
> Testing
> -------
> 
> Added unitests
> 
> 
> Thanks,
> 
> Dmytro Shkvyra
> 
>


Re: Review Request 17596: If Ganglia is not installed, server logs hundreds of error messages

Posted by Dmytro Shkvyra <ds...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17596/
-----------------------------------------------------------

(Updated Feb. 4, 2014, 8:44 p.m.)


Review request for Ambari, Mahadev Konar, Nate Cole, Tom Beerbower, and Yusaku Sako.


Changes
-------

As Tom advised I have create new implementation of Resource and add new method to it which returned evaluated config.
Now JMXPropertyProvider can get evaluated values and UI and other functionality should work as earlier.


Bugs: AMBARI-4490
    https://issues.apache.org/jira/browse/AMBARI-4490


Repository: ambari


Description
-------

The cause of error messages is that we start use new format properties like "dfs.datanode.http.address":"0.0.0.0:${ambari.dfs.datanode.http.port}".
It means that we have to replace ${ambari.dfs.datanode.http.port} with value of ambari.dfs.datanode.http.port property from current config.
In this case we need process these value references and keep in mind that:
1) We can have some references in one property, like this "dfs.datanode.http.address":"${ambari.dfs.datanode.http.host}:${ambari.dfs.datanode.http.port}"
2) Also value references can be referenced to another references.
3) Patch have to impact all configs
So, I have created private method postProcessPropertyValue in ResourceImpl.java for resolve all of value references


Diffs (updated)
-----

  ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AbstractProviderModule.java f39893c 
  ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ConfigurationResourceProvider.java 60a3780 
  ambari-server/src/main/java/org/apache/ambari/server/controller/internal/EvaluatedResourceImpl.java PRE-CREATION 
  ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ResourceImpl.java 15fb961 
  ambari-server/src/test/java/org/apache/ambari/server/controller/internal/EvaluatedResourceImplTest.java PRE-CREATION 

Diff: https://reviews.apache.org/r/17596/diff/


Testing
-------

Added unitests


Thanks,

Dmytro Shkvyra


Re: Review Request 17596: If Ganglia is not installed, server logs hundreds of error messages

Posted by Dmytro Shkvyra <ds...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17596/
-----------------------------------------------------------

(Updated Feb. 3, 2014, 6:11 p.m.)


Review request for Ambari, Mahadev Konar, Nate Cole, Tom Beerbower, and Yusaku Sako.


Changes
-------

Remove System.err


Bugs: AMBARI-4490
    https://issues.apache.org/jira/browse/AMBARI-4490


Repository: ambari


Description
-------

The cause of error messages is that we start use new format properties like "dfs.datanode.http.address":"0.0.0.0:${ambari.dfs.datanode.http.port}".
It means that we have to replace ${ambari.dfs.datanode.http.port} with value of ambari.dfs.datanode.http.port property from current config.
In this case we need process these value references and keep in mind that:
1) We can have some references in one property, like this "dfs.datanode.http.address":"${ambari.dfs.datanode.http.host}:${ambari.dfs.datanode.http.port}"
2) Also value references can be referenced to another references.
3) Patch have to impact all configs
So, I have created private method postProcessPropertyValue in ResourceImpl.java for resolve all of value references


Diffs (updated)
-----

  ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ResourceImpl.java 15fb961 
  ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ResourceImplTest.java da87bc6 

Diff: https://reviews.apache.org/r/17596/diff/


Testing
-------

Added unitests


Thanks,

Dmytro Shkvyra


Re: Review Request 17596: If Ganglia is not installed, server logs hundreds of error messages

Posted by Dmytro Shkvyra <ds...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17596/
-----------------------------------------------------------

(Updated Feb. 3, 2014, 5:48 p.m.)


Review request for Ambari, Mahadev Konar, Nate Cole, Tom Beerbower, and Yusaku Sako.


Changes
-------

Thanks Tom for your constructive criticism.
First of all - this bug exist with/without Ganglia. I have tested both cases.
About  too broadly property replacement...
Properties should be replaced only if they values exist in current properties map.
If property cant be resolved, it's value will not be replaced.
>>> I guess that a resource provider could use a different Resource implementation if that were the case.  Maybe it's okay if we really want to say that this should be the default behavior.<<<
I don't know how to manage which Resource implementation should be used in each case. Solution now it compatible with previous versions (we didn't use ${} in previous versions)
>>>One thing that I don't like is how the property value must be checked every time getProperty() is called?  Why not just do it once on setProperty()?<<<
Yes, I have fixed it as you advised. 

Also, I have added checking for cyclic property references, so it will prevent StackOverflowExeption when properties refer to each other (with any length of cycle)


Bugs: AMBARI-4490
    https://issues.apache.org/jira/browse/AMBARI-4490


Repository: ambari


Description
-------

The cause of error messages is that we start use new format properties like "dfs.datanode.http.address":"0.0.0.0:${ambari.dfs.datanode.http.port}".
It means that we have to replace ${ambari.dfs.datanode.http.port} with value of ambari.dfs.datanode.http.port property from current config.
In this case we need process these value references and keep in mind that:
1) We can have some references in one property, like this "dfs.datanode.http.address":"${ambari.dfs.datanode.http.host}:${ambari.dfs.datanode.http.port}"
2) Also value references can be referenced to another references.
3) Patch have to impact all configs
So, I have created private method postProcessPropertyValue in ResourceImpl.java for resolve all of value references


Diffs (updated)
-----

  ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ResourceImpl.java 15fb961 
  ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ResourceImplTest.java da87bc6 

Diff: https://reviews.apache.org/r/17596/diff/


Testing
-------

Added unitests


Thanks,

Dmytro Shkvyra


Re: Review Request 17596: If Ganglia is not installed, server logs hundreds of error messages

Posted by Dmytro Shkvyra <ds...@hortonworks.com>.

> On Feb. 2, 2014, 11:04 a.m., Tom Beerbower wrote:
> > I'm confused.  Are there 2 separate issues here?  What does the property token replacement have to do with Ganglia not being installed?  It seems like it would be an issue whether Ganglia was installed or not.  Could you explain?
> > 
> > On the property replacement ... If it is a requirement that the framework replace the tokens with property values from the config then I like a generic approach.  I'm just worried that you may be applying it too broadly.  Could there ever be cases where we don't want the ${} token to be replaced?  I guess that a resource provider could use a different Resource implementation if that were the case.  Maybe it's okay if we really want to say that this should be the default behavior.
> > 
> > One thing that I don't like is how the property value must be checked every time getProperty() is called?  Why not just do it once on setProperty()?
> > 
> > Also, on your Note to Nate ... I think that these reviews are public, so we probably shouldn't mention internal Jiras.

I have done some tests and reveal that properties do not replace so broadly as need. Current do not replaced property values while install configs on nodes.
Second one, it replace properties in UI, and as far as understood, this is not needed.
Please wait, I'll looking for solution.


- Dmytro


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17596/#review33393
-----------------------------------------------------------


On Feb. 3, 2014, 6:11 p.m., Dmytro Shkvyra wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17596/
> -----------------------------------------------------------
> 
> (Updated Feb. 3, 2014, 6:11 p.m.)
> 
> 
> Review request for Ambari, Mahadev Konar, Nate Cole, Tom Beerbower, and Yusaku Sako.
> 
> 
> Bugs: AMBARI-4490
>     https://issues.apache.org/jira/browse/AMBARI-4490
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> The cause of error messages is that we start use new format properties like "dfs.datanode.http.address":"0.0.0.0:${ambari.dfs.datanode.http.port}".
> It means that we have to replace ${ambari.dfs.datanode.http.port} with value of ambari.dfs.datanode.http.port property from current config.
> In this case we need process these value references and keep in mind that:
> 1) We can have some references in one property, like this "dfs.datanode.http.address":"${ambari.dfs.datanode.http.host}:${ambari.dfs.datanode.http.port}"
> 2) Also value references can be referenced to another references.
> 3) Patch have to impact all configs
> So, I have created private method postProcessPropertyValue in ResourceImpl.java for resolve all of value references
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ResourceImpl.java 15fb961 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ResourceImplTest.java da87bc6 
> 
> Diff: https://reviews.apache.org/r/17596/diff/
> 
> 
> Testing
> -------
> 
> Added unitests
> 
> 
> Thanks,
> 
> Dmytro Shkvyra
> 
>


Re: Review Request 17596: If Ganglia is not installed, server logs hundreds of error messages

Posted by Tom Beerbower <tb...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17596/#review33393
-----------------------------------------------------------


I'm confused.  Are there 2 separate issues here?  What does the property token replacement have to do with Ganglia not being installed?  It seems like it would be an issue whether Ganglia was installed or not.  Could you explain?

On the property replacement ... If it is a requirement that the framework replace the tokens with property values from the config then I like a generic approach.  I'm just worried that you may be applying it too broadly.  Could there ever be cases where we don't want the ${} token to be replaced?  I guess that a resource provider could use a different Resource implementation if that were the case.  Maybe it's okay if we really want to say that this should be the default behavior.

One thing that I don't like is how the property value must be checked every time getProperty() is called?  Why not just do it once on setProperty()?

Also, on your Note to Nate ... I think that these reviews are public, so we probably shouldn't mention internal Jiras.

- Tom Beerbower


On Jan. 31, 2014, 9:47 p.m., Dmytro Shkvyra wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17596/
> -----------------------------------------------------------
> 
> (Updated Jan. 31, 2014, 9:47 p.m.)
> 
> 
> Review request for Ambari, Mahadev Konar, Nate Cole, Tom Beerbower, and Yusaku Sako.
> 
> 
> Bugs: AMBARI-4490
>     https://issues.apache.org/jira/browse/AMBARI-4490
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> The cause of error messages is that we start use new format properties like "dfs.datanode.http.address":"0.0.0.0:${ambari.dfs.datanode.http.port}".
> It means that we have to replace ${ambari.dfs.datanode.http.port} with value of ambari.dfs.datanode.http.port property from current config.
> In this case we need process these value references and keep in mind that:
> 1) We can have some references in one property, like this "dfs.datanode.http.address":"${ambari.dfs.datanode.http.host}:${ambari.dfs.datanode.http.port}"
> 2) Also value references can be referenced to another references.
> 3) Patch have to impact all configs
> So, I have created private method postProcessPropertyValue in ResourceImpl.java for resolve all of value references
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ResourceImpl.java 15fb961 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ResourceImplTest.java da87bc6 
> 
> Diff: https://reviews.apache.org/r/17596/diff/
> 
> 
> Testing
> -------
> 
> Added unitests
> 
> 
> Thanks,
> 
> Dmytro Shkvyra
> 
>


Re: Review Request 17596: If Ganglia is not installed, server logs hundreds of error messages

Posted by Dmytro Shkvyra <ds...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17596/
-----------------------------------------------------------

(Updated Jan. 31, 2014, 9:47 p.m.)


Review request for Ambari, Mahadev Konar, Nate Cole, Tom Beerbower, and Yusaku Sako.


Changes
-------

Add Nate Cole and Tom Beerbower for reviewing this patch


Bugs: AMBARI-4490
    https://issues.apache.org/jira/browse/AMBARI-4490


Repository: ambari


Description
-------

The cause of error messages is that we start use new format properties like "dfs.datanode.http.address":"0.0.0.0:${ambari.dfs.datanode.http.port}".
It means that we have to replace ${ambari.dfs.datanode.http.port} with value of ambari.dfs.datanode.http.port property from current config.
In this case we need process these value references and keep in mind that:
1) We can have some references in one property, like this "dfs.datanode.http.address":"${ambari.dfs.datanode.http.host}:${ambari.dfs.datanode.http.port}"
2) Also value references can be referenced to another references.
3) Patch have to impact all configs
So, I have created private method postProcessPropertyValue in ResourceImpl.java for resolve all of value references


Diffs
-----

  ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ResourceImpl.java 15fb961 
  ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ResourceImplTest.java da87bc6 

Diff: https://reviews.apache.org/r/17596/diff/


Testing
-------

Added unitests


Thanks,

Dmytro Shkvyra


Re: Review Request 17596: If Ganglia is not installed, server logs hundreds of error messages

Posted by Dmytro Shkvyra <ds...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17596/
-----------------------------------------------------------

(Updated Jan. 31, 2014, 6:41 p.m.)


Review request for Ambari, Mahadev Konar and Yusaku Sako.


Changes
-------

Bug of review board, didn't save description


Bugs: AMBARI-4490
    https://issues.apache.org/jira/browse/AMBARI-4490


Repository: ambari


Description (updated)
-------

The cause of error messages is that we start use new format properties like "dfs.datanode.http.address":"0.0.0.0:${ambari.dfs.datanode.http.port}".
It means that we have to replace ${ambari.dfs.datanode.http.port} with value of ambari.dfs.datanode.http.port property from current config.
In this case we need process these value references and keep in mind that:
1) We can have some references in one property, like this "dfs.datanode.http.address":"${ambari.dfs.datanode.http.host}:${ambari.dfs.datanode.http.port}"
2) Also value references can be referenced to another references.
3) Patch have to impact all configs
So, I have created private method postProcessPropertyValue in ResourceImpl.java for resolve all of value references


Diffs
-----

  ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ResourceImpl.java 15fb961 
  ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ResourceImplTest.java da87bc6 

Diff: https://reviews.apache.org/r/17596/diff/


Testing
-------

Added unitests


Thanks,

Dmytro Shkvyra