METRON-1732: Fix job status liveness bug and parallelize finalizer file writing #1157

mmiklavc · 2018-08-10T05:18:37Z

Contributor Comments

https://issues.apache.org/jira/browse/METRON-1732

This still needs to have the # of finalizer threads option exposed for the REST application, but since it's multi-threaded code I wanted to get the review process started while I finish that part up.

Test plan and more detailed description to follow.

Pull Request Checklist

For all changes:

Is there a JIRA ticket associated with this PR? If not one needs to be created at Metron Jira.
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)?

For code changes:

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:
```
mvn -q clean integration-test install && dev-utilities/build-utils/verify_licenses.sh 
```
Have you written or updated unit tests and or integration tests to verify your changes?
n/a ~~If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under ASF 2.0?~~
Have you verified the basic functionality of the build by building and running locally with Vagrant full-dev environment or the equivalent?

For documentation related changes:

Have you ensured that format looks appropriate for the output in which it is rendered by building and verifying the site-book? If not then run the following commands and the verify changes via site-book/target/site/index.html:
```
cd site-book
mvn site
```

Note:

Please ensure that once the PR is submitted, you check travis-ci for build issues and submit an update to your PR as soon as possible.
It is also recommended that travis-ci is set up for your personal repository such that your branches are built there before submitting a pull request.

…s-write

…to account for 25% of total job progress

…lism as property in Ambari.

mmiklavc · 2018-08-11T20:58:40Z

This PR also updates the status reporting to include 25% of the progress to include the finalizer. Testing locally found that a query via the Alerts PCAP UI with page size set small (10 results per page), resulting in 7,299 pages took 15 minutes with parallelism set to 1. With parallelism set to 8 it went down to 2-3 minutes.

nickwallen · 2018-08-13T15:18:39Z

metron-interface/metron-rest/README.md

 Pcap query jobs can be configured for submission to a YARN queue.  This setting is exposed as the Spring property `pcap.yarn.queue`.  If configured, the REST application will set the `mapreduce.job.queuename` Hadoop property to that value.

+Pcap query jobs have a finalization routine that writes their results out to HDFS in pages. There is a threadpool used for this finalization that can be configured to use a specified number of threads.
+This setting is exposed as the Spring property `pcap.finalizer.threadpool.size`


Can we document the default value for this?

Do you have any advice on when a user should increase/decrease this value? Are there errors I might see that would be resolved by increasing/decreasing this value?

If you don't have a good understanding of this, then we don't need to worry about it.

Should we mention that 1C, 4C are valid values in addition to integers? Perhaps just copy the text you have in the Ambari description into the README. Good stuff.

nickwallen · 2018-08-13T15:31:53Z

metron-platform/metron-pcap/src/main/java/org/apache/metron/pcap/finalizer/PcapFinalizer.java

+  private static int getNumThreads(String numThreads) {
+    String numThreadsStr = ((String) numThreads).trim().toUpperCase();
+    if (numThreadsStr.endsWith("C")) {
+      Integer factor = Integer.parseInt(numThreadsStr.replace("C", ""));


Should we add a catch block for when a user enters an invalid value?

We could catch and provide a helpful exception message like "Invalid value for property 'pcap.finalizer.threadpool.size'; value='3CCC'". Otherwise, its kind of murky to the user what happened.

nickwallen · 2018-08-13T16:00:53Z

metron-platform/metron-pcap/src/main/java/org/apache/metron/pcap/mr/PcapJob.java

    }
-    mrJob.submit();
-    jobStatus.withState(State.SUBMITTED).withDescription("Job submitted").withJobId(mrJob.getJobID().toString());
+    synchronized (this) {


Can we add a comment about why we need the lock here?

Will do. This lock is about thread visibility as opposed to actual issues with concurrent modification. It may be that this lock is not need with getStatus being synchronized. I will double check and report back via modified code and/or code comment on this.

fyi, turns out I was right first time around. Synchronization is necessary for visibility in the timer thread that is started after these modifications. I've updated the comments in code to describe this.

nickwallen · 2018-08-13T16:17:23Z

metron-platform/metron-pcap/src/main/java/org/apache/metron/pcap/finalizer/PcapFinalizer.java

+              outFiles.add(path);
+            }
+          } catch (IOException ioe) {
+            throw new RuntimeException("Failed to write results", ioe);


Can we add the path that failed to write to the exception message?

nickwallen · 2018-08-13T16:27:11Z

metron-platform/metron-pcap/src/main/java/org/apache/metron/pcap/finalizer/PcapFinalizer.java

+    ForkJoinPool tp = new ForkJoinPool(parallelism);
+    try {
+      tp.submit(() -> {
+        toWrite.entrySet().parallelStream().forEach(e -> {


Shouldn't we be calling tp.submit for each (path, data)?

As I understand it, submit is effectively submitting the set of tasks for the parallel stream to execute within this threadpool, e.g. https://www.baeldung.com/java-8-parallel-streams-custom-threadpool. As a side note, the reason for a custom threadpool at all is so that this doesn't cause issues with other streams since the default in Java is to use a global context for this sort of thing. Liveness issues may arise when using the shared global context.

mmiklavc · 2018-08-13T17:11:00Z

Good feedback @nickwallen, I'll make adjustments.

mmiklavc · 2018-08-14T18:22:30Z

@nickwallen I've addressed your review comments. Let me know what you think.

mmiklavc · 2018-08-14T18:33:36Z

Testing

Test plan pulled from here - #1081 (comment)

Get PCAP data into Metron:

Install and setup pycapa (this has been updated in master recently) - https://github.com/apache/metron/blob/master/metron-sensors/pycapa/README.md#centos-6
(if using singlenode vagrant) Kill the enrichment, profiler, indexing, and sensor topologies via for i in bro enrichment random_access_indexing batch_indexing yaf snort;do storm kill $i;done
Start the pcap topology via $METRON_HOME/bin/start_pcap_topology.sh
Start the pycapa packet capture producer on eth1 via /usr/bin/pycapa --producer --topic pcap -i eth1 -k node1:6667
Watch the topology in the Storm UI and kill the packet capture utility from before, when the number of packets ingested is over 3k.
Ensure that at at least 3 files exist on HDFS by running hadoop fs -ls /apps/metron/pcap
Choose a file (denoted by $FILE) and dump a few of the contents using the pcap_inspector utility via $METRON_HOME//bin/pcap_inspector.sh -i $FILE -n 5
Choose one of the lines and note the protocol.
Note that when you run the commands below, the resulting file will be placed in the execution directory where you kicked off the job from.

Fixed filter

Run a fixed filter query by executing the following command with the values noted above (match your start_time format to the date format provided - default is to use millis since epoch)
$METRON_HOME/bin/pcap_query.sh fixed -st <start_time> -df "yyyyMMdd" -p <protocol_num> -rpf 500
Verify the MR job finishes successfully. Upon completion, you should see multiple files named with relatively current datestamps in your current directory, e.g. pcap-data-20160617160549737+0000.pcap
Copy the files to your local machine and verify you can them it in Wireshark. I chose a middle file and the last file. The middle file should have 500 records (per the records_per_file option), and the last one will likely have a number of records <= 500.

Query filter

Run a Stellar query filter query by executing a command similar to the following, with the values noted above (match your start_time format to the date format provided - default is to use millis since epoch)
$METRON_HOME/bin/pcap_query.sh query -st "20160617" -df "yyyyMMdd" -query "protocol == '6'" -rpf 500
Verify the MR job finishes successfully. Upon completion, you should see multiple files named with relatively current datestamps in your current directory, e.g. pcap-data-20160617160549737+0000.pcap
Copy the files to your local machine and verify you can them it in Wireshark. I chose a middle file and the last file. The middle file should have 500 records (per the records_per_file option), and the last one will likely have a number of records <= 500.

Also run riffs on the fixed query via the Metron Alerts UI PCAP query panel.

mmiklavc · 2018-08-14T18:43:52Z

Note - I also added a small blurb about pcap page size to the README.md alongside the notes on setting the finalizer threads. This was missed previously.

merrimanr · 2018-08-15T16:25:34Z

I ran a quick test in REST and it looks like the status never gets to SUCCEEDED.

Here is my request:

curl -X POST --header 'Content-Type: application/json' --header 'Accept: application/json' -d '{}' 'http://node1:8082/api/v1/pcap/fixed'

After the job finishes (looking at the RM UI), the status is:

{
  "jobId": "job_1533831319048_0046",
  "jobStatus": "FINALIZING",
  "description": "Finalizing job.",
  "percentComplete": 75,
  "pageTotal": 0
}

If I keep requesting status it always returns this.

merrimanr · 2018-08-15T16:31:10Z

Please disregard. I failed to deploy the Ambari changes correctly.

nickwallen · 2018-08-15T16:36:51Z

+1 I ran this up successfully. Validated...

Alerts visible in the UI
Metron Service Check
Capture PCAP in HDFS
Read PCAP from HDFS using CLI.
Able to open resulting pcap file with tshark -r <file>.
Read PCAP from HDFS using PCAP UI
Download PCAP from UI and open in Wireshark GUI.

mmiklavc · 2018-08-15T16:54:23Z

Awesome! Thanks for the review and smoke test @nickwallen and @merrimanr. I am going to go ahead and merge this.

…le writing (mmiklavc via mmiklavc) closes #1157

mmiklavc · 2018-08-15T17:30:56Z

Closing as merged

mmiklavc added 6 commits August 9, 2018 23:11

Parallelize finalizer writing

0b887fb

Merge with latest feature branch

5076839

Merge branch 'feature/METRON-1554-pcap-query-panel' into parallel-hdf…

1f426d4

…s-write

Add Ambari property to configure finalizer threadpool size via Ambari

0ed6016

Better handle synchronization in job status update. Change finalizer …

b1f4d86

…to account for 25% of total job progress

Resolve merge conflicts with feature branch. Expose finalizer paralle…

805d4da

…lism as property in Ambari.

nickwallen reviewed Aug 13, 2018

View reviewed changes

Address reviewer comments.

2972dfa

Resolve merge conflicts

8977a9d

asfgit pushed a commit that referenced this pull request Aug 15, 2018

METRON-1732: Fix job status liveness bug and parallelize finalizer fi…

d9e1f38

…le writing (mmiklavc via mmiklavc) closes #1157

mmiklavc closed this Aug 15, 2018

mmiklavc mentioned this pull request Mar 28, 2019

METRON-2053: Refactor metron-enrichment to decouple Storm dependencies #1368

Closed

9 tasks

mmiklavc mentioned this pull request Oct 9, 2019

METRON-2232 Upgrade to Hadoop 3.1.1 #1523

Closed

9 tasks

METRON-1732: Fix job status liveness bug and parallelize finalizer file writing #1157

METRON-1732: Fix job status liveness bug and parallelize finalizer file writing #1157

Uh oh!

Conversation

mmiklavc commented Aug 10, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Contributor Comments

Pull Request Checklist

For all changes:

For code changes:

For documentation related changes:

Note:

Uh oh!

mmiklavc commented Aug 11, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nickwallen Aug 13, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mmiklavc commented Aug 13, 2018

Uh oh!

mmiklavc commented Aug 14, 2018

Uh oh!

mmiklavc commented Aug 14, 2018

Fixed filter

Query filter

Uh oh!

mmiklavc commented Aug 14, 2018

Uh oh!

merrimanr commented Aug 15, 2018

Uh oh!

merrimanr commented Aug 15, 2018

Uh oh!

nickwallen commented Aug 15, 2018

Uh oh!

mmiklavc commented Aug 15, 2018

Uh oh!

mmiklavc commented Aug 15, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

mmiklavc commented Aug 10, 2018 •

edited

Loading

nickwallen Aug 13, 2018 •

edited

Loading