[GitHub] incubator-metron issue #188: METRON-227 (Time based flushing)

2016-07-13 Thread cestella
Github user cestella commented on the issue:

https://github.com/apache/incubator-metron/pull/188
  
Whoops, forgot, we should probably have a unit test addition given this 
functionality.


---
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 #188: METRON-227 (Time based flushing)

2016-07-13 Thread cestella
Github user cestella commented on a diff in the pull request:

https://github.com/apache/incubator-metron/pull/188#discussion_r70704170
  
--- Diff: 
metron-platform/metron-integration-test/src/main/config/zookeeper/global.json 
---
@@ -7,10 +7,12 @@
   "solr.collection": "metron",
   "solr.numShards": 1,
   "solr.replicationFactor": 1,
+  "flushIntervalInMs": "2",
+  "flushOnTimeout": "false",
--- End diff --

Do we need both parameters?  If we have a flush interval set, we should 
infer that we want to do a time-based flush, right?


---
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 #188: METRON-227 (Time based flushing)

2016-07-13 Thread ajayydv
GitHub user ajayydv opened a pull request:

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

METRON-227 (Time based flushing)

METRON-227 (Time based flushing)

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

$ git pull https://github.com/ajayydv/incubator-metron METRON-227

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

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


commit 6f091253f3d2716d0c286b2d3a232cd93a4872d2
Author: ajayydv 
Date:   2016-07-13T20:11:37Z

METRON-227 (Time based flushing)




---
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 issue #187: Metron 321

2016-07-13 Thread cestella
Github user cestella commented on the issue:

https://github.com/apache/incubator-metron/pull/187
  
+1.  In the future can you please name the PR starting with the full JIRA 
name followed by the description, like so: "METRON-321: Add items to 
.gitignore".  The full JIRA name (with the dash) triggers comments to be copied 
from the PR to the JIRA and the full description helps me know what the heck it 
is without going to the JIRA. ;)


---
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 #188: METRON-227 (Time based flushing)

2016-07-13 Thread cestella
Github user cestella commented on a diff in the pull request:

https://github.com/apache/incubator-metron/pull/188#discussion_r70703025
  
--- Diff: 
metron-platform/metron-common/src/main/java/org/apache/metron/common/writer/BulkWriterComponent.java
 ---
@@ -34,21 +34,37 @@
 import java.util.function.Function;
 
 public class BulkWriterComponent {
-  public static final Logger LOG = LoggerFactory
-.getLogger(BulkWriterComponent.class);
+  public static final Logger LOG = 
LoggerFactory.getLogger(BulkWriterComponent.class);
   private Map sensorTupleMap = new HashMap<>();
   private Map sensorMessageMap = new HashMap<>();
   private OutputCollector collector;
   private boolean handleCommit = true;
   private boolean handleError = true;
+  private Long lastFlushTime;
+  private Long flushIntervalInMs;
+  private boolean flush = false;
+
+
   public BulkWriterComponent(OutputCollector collector) {
 this.collector = collector;
+this.lastFlushTime = System.currentTimeMillis();
+this.flush = false;
   }
 
   public BulkWriterComponent(OutputCollector collector, boolean 
handleCommit, boolean handleError) {
 this(collector);
 this.handleCommit = handleCommit;
 this.handleError = handleError;
+this.lastFlushTime = System.currentTimeMillis();
+this.flush = false;
+  }
+
+  public void setFlush(boolean flush) {
--- End diff --

Since you're deciding whether to flush based on variables in the global 
config, we don't need member variables and setters for `flush` and 
`flushInterval`.  I don't like setting a member variable in the class every 
time we call write and we don't need it, they can be local variables to write


---
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 #188: METRON-227 (Time based flushing)

2016-07-13 Thread cestella
Github user cestella commented on a diff in the pull request:

https://github.com/apache/incubator-metron/pull/188#discussion_r70704522
  
--- Diff: 
metron-platform/metron-common/src/main/java/org/apache/metron/common/writer/BulkWriterComponent.java
 ---
@@ -103,10 +115,21 @@ public void write( String sensorType
 }
 messageList.add(message);
 
-if (tupleList.size() < batchSize) {
-  sensorTupleMap.put(sensorType, tupleList);
-  sensorMessageMap.put(sensorType, messageList);
-} else {
+
if(configurations.getGlobalConfig()!=null&().get(Constants.TIME_FLUSH_FLAG)!=null)
--- End diff --

I'd recommend saving off the global config in a member variable and reusing 
that reference.  It's possible the configuration will get updated between calls 
to `configurations.getGlobalConfig()` in zookeeper and therefore you'll get two 
separate configs.


---
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 issue #188: METRON-227 (Time based flushing)

2016-07-13 Thread cestella
Github user cestella commented on the issue:

https://github.com/apache/incubator-metron/pull/188
  
Great contribution, thanks!  I had a few comments, but good job. :)


---
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 issue #186: METRON-298 Remove the effective_tld_names.dat f...

2016-07-13 Thread cestella
Github user cestella commented on the issue:

https://github.com/apache/incubator-metron/pull/186
  
Just for posterity, I ran it up in vagrant


---
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 issue #186: METRON-298 Remove the effective_tld_names.dat f...

2016-07-13 Thread dlyle65535
Github user dlyle65535 commented on the issue:

https://github.com/apache/incubator-metron/pull/186
  
I've been trying, but it's AWS is fighting me.


---
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.
---