[GitHub] incubator-metron pull request #424: METRON-672: SolrIndexingIntegrationTest ...

2017-01-26 Thread asfgit
Github user asfgit closed the pull request at:

https://github.com/apache/incubator-metron/pull/424


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] incubator-metron pull request #424: METRON-672: SolrIndexingIntegrationTest ...

2017-01-26 Thread cestella
GitHub user cestella reopened a pull request:

https://github.com/apache/incubator-metron/pull/424

METRON-672: SolrIndexingIntegrationTest fails intermittently

This failure is due to a change in default behavior when indexing was split 
off into a separate configuration file.  The default batch size was changed 
from `5` to `1` in particular.  This, by itself, is not a problem, but the 
`IndexingIntegrationTest` (base class for Solr and Elastic search integration 
tests):
* submits the configs
* starts the indexing topology
* writes the input data

The writing of the input data may happen before the topology fully loads or 
the configuration fully loads, especially if the machine running the unit tests 
is under load (like with travis).  As a result, the first record may end up 
with the default batch size (of 1) and write out immediately because the 
indexing configs haven't loaded into zookeeper just yet.  In that circumstance, 
eventually the configs load and the batch size is set to `5`.  Meanwhile we've 
written 10 records and are expecting 10 in return, but because you wrote the 
first out already and then the next 5, we have another 4 pending to be written 
by the `BulkMessageWriterBolt`.

So, the failure scenario is as follows:
* Message 1 is received and the indexing config hasn't loaded yet, so the 
batch size is 1 and it immediately gets written out
* Message 2 - 5 are each received and the indexing config has loaded, so 
the batch size is 5 and it queues up
* Message 6 is received and the batch writes out
* Messages 7 - 10 are received, but never make a full batch, so we time out 
waiting for them to write out

The fix is to ensure that we don't write out messages to kafka until the 
configs are loaded, which is what this PR does.

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

$ git pull https://github.com/cestella/incubator-metron METRON-672

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

https://github.com/apache/incubator-metron/pull/424.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 #424


commit 9617e099d79920b1ffd40555c036f3826ea96e6b
Author: cstella 
Date:   2017-01-26T00:01:21Z

Fixing integration test.

commit 5e836f4eaba51783c2a699e41aba6568aa6a5f99
Author: cstella 
Date:   2017-01-26T00:52:36Z

Merge branch 'master' into METRON-672

commit b2103fdb735854a375a70809e625044f0f200a29
Author: cstella 
Date:   2017-01-26T01:12:21Z

Updating to react to comments.

commit 0f6602ccf86c0cb68d851ab5337693346bb284e4
Author: cstella 
Date:   2017-01-26T01:25:09Z

fixed commit.




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] incubator-metron pull request #424: METRON-672: SolrIndexingIntegrationTest ...

2017-01-26 Thread cestella
Github user cestella closed the pull request at:

https://github.com/apache/incubator-metron/pull/424


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] incubator-metron pull request #424: METRON-672: SolrIndexingIntegrationTest ...

2017-01-25 Thread justinleet
Github user justinleet commented on a diff in the pull request:

https://github.com/apache/incubator-metron/pull/424#discussion_r97915042
  
--- Diff: 
metron-platform/metron-enrichment/src/test/java/org/apache/metron/enrichment/integration/components/ConfigUploadComponent.java
 ---
@@ -38,6 +39,7 @@
   private String enrichmentConfigsPath;
   private String indexingConfigsPath;
   private String profilerConfigPath;
+  private Optional> 
postStartCallback = Optional.empty();
--- End diff --

Could this just use Consumer instead of Function? Since the second type 
parameter is Void, it seems like the Function is just being a Consumer anyway


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] incubator-metron pull request #424: METRON-672: SolrIndexingIntegrationTest ...

2017-01-25 Thread justinleet
Github user justinleet commented on a diff in the pull request:

https://github.com/apache/incubator-metron/pull/424#discussion_r97914890
  
--- Diff: 
metron-platform/metron-indexing/src/test/java/org/apache/metron/indexing/integration/IndexingIntegrationTest.java
 ---
@@ -184,6 +203,26 @@ public void test() throws Exception {
 }
   }
 
+  private void waitForIndex(String zookeeperQuorum) throws Exception {
+try(CuratorFramework client = getClient(zookeeperQuorum)) {
+  client.start();
+  byte[] bytes = null;
+  do {
+try {
+  bytes = 
ConfigurationsUtils.readSensorIndexingConfigBytesFromZookeeper(testSensorType, 
client);
+  Thread.sleep(1000);
+}
+catch(KeeperException.NoNodeException nne) {
+  //kindly ignore because the path might not exist just yet.
+}
+  }
+  while(bytes == null || bytes.length == 0);
+  return;
--- End diff --

Drop the return, since it's a void method.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] incubator-metron pull request #424: METRON-672: SolrIndexingIntegrationTest ...

2017-01-25 Thread justinleet
Github user justinleet commented on a diff in the pull request:

https://github.com/apache/incubator-metron/pull/424#discussion_r97914854
  
--- Diff: 
metron-platform/metron-indexing/src/test/java/org/apache/metron/indexing/integration/IndexingIntegrationTest.java
 ---
@@ -139,11 +146,22 @@ public void test() throws Exception {
   inputDocs.add(m);
 
 }
+final AtomicBoolean isLoaded = new AtomicBoolean(false);
 ConfigUploadComponent configUploadComponent = new 
ConfigUploadComponent()
 .withTopologyProperties(topologyProperties)
 .withGlobalConfigsPath(TestConstants.SAMPLE_CONFIG_PATH)
 .withEnrichmentConfigsPath(TestConstants.SAMPLE_CONFIG_PATH)
 .withIndexingConfigsPath(TestConstants.SAMPLE_CONFIG_PATH)
+.withPostStartCallback(component -> {
+  try {
+
waitForIndex(component.getTopologyProperties().getProperty(ZKServerComponent.ZOOKEEPER_PROPERTY));
+  } catch (Exception e) {
+e.printStackTrace();
+  }
+  isLoaded.set(true);
+  return null;
+  }
+);
 ;
--- End diff --

Can you kill the extra semicolon?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] incubator-metron pull request #424: METRON-672: SolrIndexingIntegrationTest ...

2017-01-25 Thread cestella
GitHub user cestella opened a pull request:

https://github.com/apache/incubator-metron/pull/424

METRON-672: SolrIndexingIntegrationTest fails intermittently

This failure is due to a change in default behavior when indexing was split 
off into a separate configuration file.  The default batch size was changed 
from `5` to `1` in particular.  This, by itself, is not a problem, but the 
`IndexingIntegrationTest` (base class for Solr and Elastic search integration 
tests):
* submits the configs
* starts the indexing topology
* writes the input data

The writing of the input data may happen before the topology fully loads or 
the configuration fully loads, especially if the machine running the unit tests 
is under load (like with travis).  As a result, the first record may end up 
with the default batch size (of 1) and write out immediately because the 
indexing configs haven't loaded into zookeeper just yet.  In that circumstance, 
eventually the configs load and the batch size is set to `5`.  Meanwhile we've 
written 10 records and are expecting 10 in return, but because you wrote the 
first out already and then the next 5, we have another 4 pending to be written 
by the `BulkMessageWriterBolt`.

So, the failure scenario is as follows:
* Message 1 is received and the indexing config hasn't loaded yet, so the 
batch size is 1 and it immediately gets written out
* Message 2 - 5 are each received and the indexing config has loaded, so 
the batch size is 5 and it queues up
* Message 6 is received and the batch writes out
* Messages 7 - 10 are received, but never make a full batch, so we time out 
waiting for them to write out

The fix is to ensure that we don't write out messages to kafka until the 
configs are loaded, which is what this PR does.

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

$ git pull https://github.com/cestella/incubator-metron METRON-672

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

https://github.com/apache/incubator-metron/pull/424.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 #424






---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---