-
Notifications
You must be signed in to change notification settings - Fork 506
METRON-1641: Enable Pcap jobs to be submitted asynchronously #1081
METRON-1641: Enable Pcap jobs to be submitted asynchronously #1081
Conversation
|
Testing Get PCAP data into Metron:
Fixed filter
Query filter
|
cestella
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like this! I especially like the statusable abstraction here. Good job; I'm +1 after the full-dev testing checkbox is checked and the small comment I had.
metron-platform/pom.xml
Outdated
| <module>metron-enrichment</module> | ||
| <module>metron-solr</module> | ||
| <module>metron-parsers</module> | ||
| <module>metron-job</module> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we adjust the indentation here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Heh, the pom.xml has tabs instead of spaces. Rather than reformat everything in the file I just changed that line use tabs.
|
Looks good! One thing I'm trying to wrap my head around is how we get status if we only have a job id or unique identifier for a job? JobStatus doesn't have an id so I'm assuming resultPath is the unique identifier here. As far as I can tell an instance of org.apache.hadoop.mapreduce.Job is kept in memory and is responsible for reporting status. I can think of a couple scenarios where this might be problematic. |
|
I think for a first cut, it's ok to have the restrictions that:
|
|
One thing I didn't see. Can we make sure we pass along the yarn queue to the job? |
|
One more comment about restartability, I think we could potentially support this with this architecture in the future. You can recover the Job object from MR via the JobClient We could look for jobs which are completed but not in the HDFS structure and recover them on REST start. I would suggest doing that as a follow-on though. |
|
Perfect. That addresses my concern. Doing this in a follow on is fine since it's not necessary when using the CLI. |
|
I spun this up in full dev, ran a fixed query, and got this error: Looks like the MR jobs succeeded but partitioning the files to the local FS did not work. |
|
Good catch on the FS @merrimanr - also finding that via manual testing. I believe I have a workaround that degrades nicely to the configuration default and also allows you to pass in the scheme in the path. |
|
Also @merrimanr agreed about the points you made regarding restart-ability, etc. in the long run. In the short term, @cestella has done a rather good job of summarizing my thoughts on a v1 pass at this feature set. I will:
How's that sound to you both? |
|
Heads up, hadoop config class is where you set queue name iirc. We already pass that in as an arg. This would simply need to be provided via the calling job manager class. |
|
That sounds good to me. |
|
I tested this and it's working for me in full dev. I think it's good enough to go into the feature branch. +1 |
|
Can we merge this? Any other items you would like addressed @cestella? |
|
+1, lgtm |
|
@mmiklavc can you merge and close this PR? |
|
Committed to the feature branch. |
Contributor Comments
https://issues.apache.org/jira/browse/METRON-1641
This enables Pcap Jobs to be run asynchronously. The PcapJob class itself is now a Statusable implementation that allows clients to poll for current JobStatus. This implementation exposes the new functionality on the job class but keeps the existing PcapCli functionality intact and unchanged. The tests for this will be in a comment below, taken from #256.
This validation should check that the current pcap functionality does not break. Follow on PR's will leverage the new asynchronous capabilities.
Pull Request Checklist
Thank you for submitting a contribution to Apache Metron.
Please refer to our Development Guidelines for the complete guide to follow for contributions.
Please refer also to our Build Verification Guidelines for complete smoke testing guides.
In order to streamline the review of the contribution we ask you follow these guidelines and ask you to double check the following:
For all changes:
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:
Have you written or updated unit tests and or integration tests to verify your changes?
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: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.