You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@metron.apache.org by nickwallen <gi...@git.apache.org> on 2018/04/18 16:02:56 UTC

[GitHub] metron pull request #997: METRON-1529 CONFIG_GET Fails to Retrieve Latest Co...

GitHub user nickwallen opened a pull request:

    https://github.com/apache/metron/pull/997

    METRON-1529 CONFIG_GET Fails to Retrieve Latest Config When Run in Zeppelin REPL

    The cache used to back the `CONFIG_GET` does not update when changes are made in Zookeeper.  This problem only occurs when the REPL is run in Zeppelin.  It does not occur when the REPL is run in the CLI.  [METRON-1529](https://issues.apache.org/jira/browse/METRON-1529) includes step to replicate the issue.
    
    **NOTE:** This change is dependent on #982 .  It should only be merged after #982.
    
    ## Changes
    
    The root cause is not clear, but must be due to the way in which Zeppelin launches the interpreters and keeps them isolated from the core of Zeppelin itself.  
    
    That being said, there really is no need for a cache to back `CONFIG_GET`.  The function should always go to Zookeeper to get the latest configuration values.  This simplifies the implementation and solves the problem at-hand.
    
    The changes relevant to this PR include the following.
    
    * [`metron-platform/metron-management/src/main/java/org/apache/metron/management/ConfigurationFunctions.java`](https://github.com/apache/metron/compare/master...nickwallen:METRON-1529?expand=1#diff-00b9178e2e80b694bac96e387f434ec9)
    
    * [`metron-platform/metron-management/src/test/java/org/apache/metron/management/ConfigurationFunctionsTest.java`](https://github.com/apache/metron/compare/master...nickwallen:METRON-1529?expand=1#diff-187b5eaaf0be7dc001aaa47388444036)
    
    * [`metron-platform/metron-common/src/main/java/org/apache/metron/common/configuration/ConfigurationsUtils.java`](https://github.com/apache/metron/compare/master...nickwallen:METRON-1529?expand=1#diff-b8d2f6514e881628a94f49e342bc7be)
    
    ## Testing
    
    * The unit tests were enhanced to validate this change.  
    
    * I have validate this change in the CentOS dev environment
    
    * I have tested this in the Zeppelin REPL by following the steps outlined in [METRON-1529](https://issues.apache.org/jira/browse/METRON-1529).
    
        ![screen shot 2018-04-18 at 11 51 06 am](https://user-images.githubusercontent.com/2475409/38943392-48370022-42ff-11e8-9485-55ee7f67c7a5.png)
    
    * I have tested this in the CLI REPL by launching the REPL using the following command and then running the same set of Stellar commands.
    
            java -cp "metron-stellar/stellar-common/target/stellar-common-0.4.3.jar:metron-platform/metron-management/target/metron-management-0.4.3.jar:metron-platform/metron-parsers/target/metron-parsers-0.4.3-uber.jar:~/.m2/repository/org/slf4j/slf4j-simple/1.7.9/slf4j-simple-1.7.25.jar"  org.apache.metron.stellar.common.shell.cli.StellarShell -z localhost:2181
    
        ![screen shot 2018-04-18 at 11 54 21 am](https://user-images.githubusercontent.com/2475409/38943413-57e2608e-42ff-11e8-8937-6fdf63c7ecf9.png)
    
    
    
    
    ## Pull Request Checklist
    
    - [ ] Is there a JIRA ticket associated with this PR? If not one needs to be created at [Metron Jira](https://issues.apache.org/jira/browse/METRON/?selectedTab=com.atlassian.jira.jira-projects-plugin:summary-panel).
    - [ ] Does your PR title start with METRON-XXXX where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character.
    - [ ] Has your PR been rebased against the latest commit within the target branch (typically master)?
    - [ ] Have you included steps to reproduce the behavior or problem that is being changed or addressed?
    - [ ] Have you included steps or a guide to how the change may be verified and tested manually?
    - [ ] Have you ensured that the full suite of tests and checks have been executed in the root metron folder via:
    - [ ] Have you written or updated unit tests and or integration tests to verify your changes?
    - [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)?
    - [ ] Have you verified the basic functionality of the build by building and running locally with Vagrant full-dev environment or the equivalent?


You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/nickwallen/metron METRON-1529

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/metron/pull/997.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #997
    
----
commit e1556ee59014d00c8e24ceaf68b94b0df4d449ec
Author: nickwallen <ni...@...>
Date:   2018-04-17T13:43:16Z

    METRON-1529 CONFIG_GET Fails to Retrieve Latest Config When Run in Zeppelin REPL

commit 5e08e28b4f2c67679199fe0bf4af3b66c99630e6
Author: Nick Allen <ni...@...>
Date:   2018-04-18T15:36:15Z

    Merge remote-tracking branch 'apache/master' into METRON-1529

----


---

[GitHub] metron pull request #997: METRON-1529 CONFIG_GET Fails to Retrieve Latest Co...

Posted by nickwallen <gi...@git.apache.org>.
Github user nickwallen commented on a diff in the pull request:

    https://github.com/apache/metron/pull/997#discussion_r182503862
  
    --- Diff: metron-stellar/stellar-common/src/main/java/org/apache/metron/stellar/common/utils/StellarProcessorUtils.java ---
    @@ -1,34 +1,31 @@
     /**
    - * Licensed to the Apache Software Foundation (ASF) under one
    - * or more contributor license agreements.  See the NOTICE file
    - * distributed with this work for additional information
    - * regarding copyright ownership.  The ASF licenses this file
    - * to you under the Apache License, Version 2.0 (the
    - * "License"); you may not use this file except in compliance
    - * with the License.  You may obtain a copy of the License at
    + * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
    --- End diff --
    
    Fixed. Thanks


---

[GitHub] metron pull request #997: METRON-1529 CONFIG_GET Fails to Retrieve Latest Co...

Posted by nickwallen <gi...@git.apache.org>.
Github user nickwallen closed the pull request at:

    https://github.com/apache/metron/pull/997


---

[GitHub] metron pull request #997: METRON-1529 CONFIG_GET Fails to Retrieve Latest Co...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/metron/pull/997


---

[GitHub] metron pull request #997: METRON-1529 CONFIG_GET Fails to Retrieve Latest Co...

Posted by mmiklavc <gi...@git.apache.org>.
Github user mmiklavc commented on a diff in the pull request:

    https://github.com/apache/metron/pull/997#discussion_r182497608
  
    --- Diff: metron-stellar/stellar-common/src/main/java/org/apache/metron/stellar/common/utils/StellarProcessorUtils.java ---
    @@ -1,34 +1,31 @@
     /**
    - * Licensed to the Apache Software Foundation (ASF) under one
    - * or more contributor license agreements.  See the NOTICE file
    - * distributed with this work for additional information
    - * regarding copyright ownership.  The ASF licenses this file
    - * to you under the Apache License, Version 2.0 (the
    - * "License"); you may not use this file except in compliance
    - * with the License.  You may obtain a copy of the License at
    + * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
    --- End diff --
    
    Probably don't want to change the license header format on this - can you revert to the normal formatting? e.g. https://github.com/nickwallen/metron/blob/5e08e28b4f2c67679199fe0bf4af3b66c99630e6/metron-stellar/stellar-common/src/main/java/org/apache/metron/stellar/common/shell/DefaultStellarShellExecutor.java


---

[GitHub] metron issue #997: METRON-1529 CONFIG_GET Fails to Retrieve Latest Config Wh...

Posted by cestella <gi...@git.apache.org>.
Github user cestella commented on the issue:

    https://github.com/apache/metron/pull/997
  
    +1 by inspection, great job!


---

[GitHub] metron pull request #997: METRON-1529 CONFIG_GET Fails to Retrieve Latest Co...

Posted by nickwallen <gi...@git.apache.org>.
GitHub user nickwallen reopened a pull request:

    https://github.com/apache/metron/pull/997

    METRON-1529 CONFIG_GET Fails to Retrieve Latest Config When Run in Zeppelin REPL

    The cache used to back the `CONFIG_GET` function does not update when changes are made in Zookeeper.  This problem only occurs when the REPL is run in Zeppelin.  It does not occur when the REPL is run in the CLI.  [METRON-1529](https://issues.apache.org/jira/browse/METRON-1529) includes step to replicate the issue.
    
    **NOTE:** This change is dependent on #982 .  It should only be merged after #982.
    
    ## Changes
    
    The root cause is not clear, but must be due to the way in which Zeppelin launches the interpreters and keeps them isolated from the core of Zeppelin itself.  
    
    That being said, there really is no need for a cache to back `CONFIG_GET`.  The function should always go to Zookeeper to get the latest configuration values.  This simplifies the implementation and solves the problem at-hand.
    
    The changes relevant to this PR include the following.
    
    * [`metron-platform/metron-management/src/main/java/org/apache/metron/management/ConfigurationFunctions.java`](https://github.com/apache/metron/compare/master...nickwallen:METRON-1529?expand=1#diff-00b9178e2e80b694bac96e387f434ec9)
    
    * [`metron-platform/metron-management/src/test/java/org/apache/metron/management/ConfigurationFunctionsTest.java`](https://github.com/apache/metron/compare/master...nickwallen:METRON-1529?expand=1#diff-187b5eaaf0be7dc001aaa47388444036)
    
    * [`metron-platform/metron-common/src/main/java/org/apache/metron/common/configuration/ConfigurationsUtils.java`](https://github.com/apache/metron/compare/master...nickwallen:METRON-1529?expand=1#diff-b8d2f6514e881628a94f49e342bc7be)
    
    ## Testing
    
    * The unit tests were enhanced to validate this change.  
    
    * I have validate this change in the CentOS dev environment
    
    * I have tested this in the Zeppelin REPL by following the steps outlined in [METRON-1529](https://issues.apache.org/jira/browse/METRON-1529).
    
        ![screen shot 2018-04-18 at 11 51 06 am](https://user-images.githubusercontent.com/2475409/38943392-48370022-42ff-11e8-9485-55ee7f67c7a5.png)
    
    * I have tested this in the CLI REPL by launching the REPL using the following command and then running the same set of Stellar commands.
    
            java -cp "metron-stellar/stellar-common/target/stellar-common-0.4.3.jar:metron-platform/metron-management/target/metron-management-0.4.3.jar:metron-platform/metron-parsers/target/metron-parsers-0.4.3-uber.jar:~/.m2/repository/org/slf4j/slf4j-simple/1.7.9/slf4j-simple-1.7.25.jar"  org.apache.metron.stellar.common.shell.cli.StellarShell -z localhost:2181
    
        ![screen shot 2018-04-18 at 11 54 21 am](https://user-images.githubusercontent.com/2475409/38943413-57e2608e-42ff-11e8-8937-6fdf63c7ecf9.png)
    
    
    
    
    ## Pull Request Checklist
    
    - [x] Is there a JIRA ticket associated with this PR? If not one needs to be created at [Metron Jira](https://issues.apache.org/jira/browse/METRON/?selectedTab=com.atlassian.jira.jira-projects-plugin:summary-panel).
    - [x] Does your PR title start with METRON-XXXX where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character.
    - [x] Has your PR been rebased against the latest commit within the target branch (typically master)?
    - [x] Have you included steps to reproduce the behavior or problem that is being changed or addressed?
    - [x] Have you included steps or a guide to how the change may be verified and tested manually?
    - [x] Have you ensured that the full suite of tests and checks have been executed in the root metron folder via:
    - [x] Have you written or updated unit tests and or integration tests to verify your changes?
    - [x] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)?
    - [x] Have you verified the basic functionality of the build by building and running locally with Vagrant full-dev environment or the equivalent?


You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/nickwallen/metron METRON-1529

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/metron/pull/997.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #997
    
----
commit e1556ee59014d00c8e24ceaf68b94b0df4d449ec
Author: nickwallen <ni...@...>
Date:   2018-04-17T13:43:16Z

    METRON-1529 CONFIG_GET Fails to Retrieve Latest Config When Run in Zeppelin REPL

commit 5e08e28b4f2c67679199fe0bf4af3b66c99630e6
Author: Nick Allen <ni...@...>
Date:   2018-04-18T15:36:15Z

    Merge remote-tracking branch 'apache/master' into METRON-1529

commit 37a6ba5c0c69a374cfa3f5338ec106702411c47f
Author: Nick Allen <ni...@...>
Date:   2018-04-18T17:11:44Z

    Revert changes to SensorParserConfig.java as not relevant to this PR

commit 08a192ac564939adf45f50f1ebf352aba907d3a9
Author: Nick Allen <ni...@...>
Date:   2018-04-18T17:13:01Z

    Revert changes to test.json as not relevant to this PR

commit 0cda899d1b4d40619195ba3c2510afd9039e2c0a
Author: Nick Allen <ni...@...>
Date:   2018-04-18T17:15:24Z

    Revert changes to license header of StellarProcessorUtils.java

commit fa235f844c497517a3793460d7b18319b6b18e4d
Author: Nick Allen <ni...@...>
Date:   2018-04-18T21:11:40Z

    Updated CONFIG_GET so both the GLOBALS and PROFILER adhere to the 'emptyIfNotPresent' arg, just like the other types

commit c961ea2aa70bf79c71bbedae787ac0f74e598d45
Author: Nick Allen <ni...@...>
Date:   2018-04-19T20:06:59Z

    METRON-1529 Web Start Scripts Incorrectly Report OK If Process Dies

commit fffa2554f5b7d6915a21cf6ad27d891a6293b978
Author: Nick Allen <ni...@...>
Date:   2018-04-19T20:07:31Z

    Revert "METRON-1529 Web Start Scripts Incorrectly Report OK If Process Dies"
    
    This reverts commit c961ea2aa70bf79c71bbedae787ac0f74e598d45.

----


---