Skip to content
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

Zip Archive Abstraction plus Minor tweaks for factor file generator #2416

Closed
wants to merge 5 commits into from

Conversation

mchandschuh
Copy link
Contributor

Description

Here's the zip archive abstraction I spoke about. The main entry is through Archive.OpenRead and Archive.OpenWrite and you can pass it an enum value picking which underlying implementation will be used -- figured it may be handy -- factor file generator set up to pull from command line or config if value present. A super awesome thing in here is the ArchiveCache which allows you to 'checkout' a zip instance and 'return' it -- like it's a freakin' library book! This support lockless multi-threaded read scenarios.

Related Issue

See #2378

Motivation and Context

These changes are in support of the new factor file generation/supplementation/projection application.

Requires Documentation Change

No.

How Has This Been Tested?

Probably not well enough, really need to see days tick by w/ the ticker plant properly emitting non-finalized events.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Non-functional change (xml comments/documentation/etc)

Checklist:

  • My code follows the code style of this project.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.
  • My branch follows the naming convention bug-<issue#>-<description or feature-<issue#>-<description>

Adds FirstTicker to MapFile
Adds MostRecentFactorChange to FactorFile
Minor clean up in StreamReaderEnumerable
Make Dividend.ReferencePrice settable
Splits factors are recorded in the factor file as the previous
trading day and not the actual day of the split.
ArchiveCache allows for lockless concurrent reads of zip files
ArchiveImplementation provides for easy swapping of underlying
implementation in case of issues running on one system or another.
@mchandschuh mchandschuh force-pushed the feature-2378-generator-factors branch from 1ad94f2 to e308ce7 Compare August 22, 2018 15:16
Copy link
Member

@Martin-Molinero Martin-Molinero left a comment

Choose a reason for hiding this comment

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

Very cool!

It was 'possible' for a race to exist between disposal of a
cache entry and returning an instance to that cache entry.
By removing first, the race on the returned entry will simply
create a new cache entry and the old entry will be disposed.
The thinking is that this has the possibility to lead to memory leaks.
@jaredbroad
Copy link
Member

There are core issues in the application of mono zip libraries which cause corrupt zips and thread explosions which aren't fully understood; and rather than understanding them fully we've pulled in three separate zip libraries and applied them in various ways. I think to merge this is building layers on top of that poor initial decision and cleaning up the compression libraries should be done in a more thoughtful way disconnected from the Live-Corporate events issue.

So as tempting as it is to merge it to continue the march of progress I know we'll just be installing bad patterns for the future that we'll need to undo. And rather than debating the zip implementation here; I'll close this for now as too much code unrelated to the Live-Corporate events issue and open issue (#2435) to continue archive clean up later.

@jaredbroad jaredbroad closed this Aug 22, 2018
@jaredbroad jaredbroad deleted the feature-2378-generator-factors branch November 23, 2019 01:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants