Skip to content
This repository has been archived by the owner on Mar 9, 2022. It is now read-only.

Rejection of invalid timestamps #43

Merged
merged 3 commits into from
Mar 13, 2018
Merged

Rejection of invalid timestamps #43

merged 3 commits into from
Mar 13, 2018

Conversation

mjs
Copy link
Contributor

@mjs mjs commented Mar 6, 2018

A new "max_time_delta_secs" configuration option has been added. If the filter sees a line with a timestamp which is outside ±max_time_delta_secs around the current time it will be reject. A new metric has been added to the filter which counts these rejected lines.

This feature adds a 3-5% performance hit to the filter ProcessBatch benchmark, although this is largely lost when compared to the cost of matching any realistic number of filter rules (i.e. regex matching is expensive).

Fixes #27.

Without doing this benchmark numbers of successive runs get merged
together, with confusing/hilarious results.
w.processLine(line)
} else {
w.stats.Inc(linesInvalidTime)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should add printing the failed line here if we are in debug mode. Hunting down the offender becomes much easier.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

mjs added 2 commits March 13, 2018 10:49
A new "max_time_delta_secs" configuration option has been added. If
the filter sees a line with a timestamp which is outside
±max_time_delta_secs around the current time it will be reject. A new
metric has been added to the filter which counts these rejected lines.

This feature adds a 3-5% performance hit to the filter ProcessBatch
benchmark, although this is largely lost when compared to the cost of
matching any realistic number of filter rules (i.e. regex matching is
expensive).

Fixes #27.
This revision is the same as before except it has timestamps on the
input lines, aligning it with the benchmark in master.
@mjs mjs changed the title WIP: Rejection of invalid timestamps Rejection of invalid timestamps Mar 12, 2018
@mjs mjs removed the do not merge label Mar 12, 2018
Copy link
Contributor

@oplehto oplehto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@oplehto oplehto merged commit a67dc47 into jumptrading:master Mar 13, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants