-
Notifications
You must be signed in to change notification settings - Fork 64
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
zqd: Support pcap ingest for archive stores #1450
Conversation
5f8a9c2
to
661152b
Compare
661152b
to
45f000e
Compare
Because of differences in how an archive ingests data implement new ingest.PcapOp (ingest.archivePcapOp) for archive-backed spaces. In order to take advantage of the streaming nature of ingest, abandon the brute force snapshort approach taken with file stores. Instead, temporary log files created by each pcap analyzer is are tailed and written to the archive store as new data arrives (see fs.TFile and Fs.DirWatcher). As a result for pcap ingest on archive stores, new data should constantly arriving. In order to continue pcap ingest support for soon-to-be deprecated filestore spaces, keep around existing ingest.PcapOp (renamed to ingest.legacyPcapOp). closes #1132
45f000e
to
55095e1
Compare
3a2c8f7
to
6869f9d
Compare
I'm noticing a race with |
1802110
to
e2142bd
Compare
e2142bd
to
82eafe7
Compare
d8ce701
to
420db3b
Compare
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.
Before this gets merged, I'd like to try this out with Brim, by temporarily making archive store the default.
return atomic.LoadInt64(&r.recordsRead) | ||
} | ||
|
||
// Snap for archivePcapOp is functionally useless. It is only here to satisfy |
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.
When we can run Brim against this branch, I'm curious how this impacts the live updating. I thought Brim triggered updating its view during pcap by looking for new snapshots, but I haven't looked at that code in while.
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.
As discussed offline, it is. Without an incrementing snapshot brim will not know new data is available to query. As discussed, this should really be keeping track of record count (and bytes) written to the archive, return it in the status updates, and Brim should use this to establish if new data is available. Will file an issue.
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.
dr *logTailer | ||
} | ||
|
||
func TestLogTailer(t *testing.T) { |
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.
Just want to register that I'm +1 on using testify/suite.
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.
It is definitely very useful for certain circumstances.
Co-authored-by: Alfred Landrum <[email protected]>
Co-authored-by: Noah Treuhaft <[email protected]>
Co-authored-by: Noah Treuhaft <[email protected]>
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 pulled this branch, merged to master, altered zqd to choose archive store by default, and then ran Brim against it. I then uploaded the 'all.pcap' file & watched the archive tsdir's get created during import. (as noted, the live updates in Brim aren't working yet, but that will happen soon via #1488 ). I was able to verify that the resulting archive had the same count as the doing the process with the filestore. So 👍 from me.
Thanks for the thorough review @alfred-landrum
You mean #1488, correct? |
oops, yes, fixed the comment! |
Because of differences in how an archive ingests data implement
new ingest.PcapOp (ingest.archivePcapOp) for archive-backed spaces.
In order to take advantage of the streaming nature of ingest, abandon
the brute force snapshort approach taken with file stores. Instead,
temporary log files created by each pcap analyzer is are tailed and
written to the archive store as new data arrives (see fs.TFile and
Fs.DirWatcher). As a result for pcap ingest on archive stores, new
data should constantly arriving.
In order to continue pcap ingest support for soon-to-be deprecated
filestore spaces, keep around existing ingest.PcapOp (renamed to
ingest.legacyPcapOp).
closes #1132