Skip to content
This repository has been archived by the owner on Aug 18, 2020. It is now read-only.

[CBR-446] Evaluate acid-state fix #3650

Merged
merged 2 commits into from
Oct 1, 2018
Merged

[CBR-446] Evaluate acid-state fix #3650

merged 2 commits into from
Oct 1, 2018

Conversation

parsonsmatt
Copy link
Contributor

@parsonsmatt parsonsmatt commented Sep 21, 2018 •
edited
Loading

Description

There was a space leak in acid-state. This PR incorporates that fix and removes the checkpointing code that was a temporary in-repo fix.

Linked issue

Type of change

  • 🐞 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)
  • [~] 🏭 Refactoring that does not change existing functionality but does improve things like code readability, structure etc
  • [~] 🔨 New or improved tests for existing code
  • [~] ⛑ git-flow chore (backport, hotfix, etc)

Developer checklist

  • I have read the style guide document, and my code follows the code style of this project.
  • If my code deals with exceptions, it follows the guidelines.
  • I have updated any documentation accordingly, if needed. Documentation changes can be reflected in opening a PR on cardanodocs.com, amending the inline Haddock comments, any relevant README file or one of the document listed in the docs directory.
  • CHANGELOG entry has been added and is linked to the correct PR on GitHub.

Testing checklist

  • [~] I have added tests to cover my changes.
  • All new and existing tests passed.

QA Steps

Screenshots (if available)

@parsonsmatt parsonsmatt force-pushed the matt/cbr-446/test-acid-state branch 2 times, most recently from 9b25827 to 39a136a Compare September 28, 2018 20:55
@adinapoli-iohk
Copy link
Contributor

screen shot 2018-09-29 at 08 46 14

Looks great, thanks @parsonsmatt !

adinapoli-iohk
adinapoli-iohk approved these changes Sep 29, 2018
View reviewed changes
Copy link
Contributor

@adinapoli-iohk adinapoli-iohk left a comment

Choose a reason for hiding this comment

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

LGTM, @parsonsmatt ! Let's fix stack2nix and let's merge it!

@KtorZ
Copy link
Contributor

KtorZ commented Oct 1, 2018

I'll rebase this and fix stack2nix while Matt is asleep :)

parsonsmatt and others added 2 commits October 1, 2018 14:05
@parsonsmatt @KtorZ
[CBR-446] Use acid-state fix
e87e9ba 
- uses a temporary fork of acid-state which incorporates a fix for a
  memory leak when the process goes for a long time without
  checkpointing
- Removes the checkpointing process in the BListener codebase, as that
  was a work-around for the acid-state problem.
@KtorZ
[CBR-446] Re-generate pkgs/default.nix
1c6acee
@KtorZ KtorZ force-pushed the matt/cbr-446/test-acid-state branch from 39a136a to 1c6acee Compare October 1, 2018 12:06
@KtorZ
Copy link
Contributor

KtorZ commented Oct 1, 2018

Integration tests still failing, fix coming as part of #3684, likely not related to this.

@KtorZ KtorZ merged commit bd0b7ea into develop Oct 1, 2018
@KtorZ KtorZ deleted the matt/cbr-446/test-acid-state branch October 1, 2018 15:29
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@adinapoli-iohk adinapoli-iohk adinapoli-iohk approved these changes

@matt-noonan matt-noonan Awaiting requested review from matt-noonan

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@parsonsmatt @adinapoli-iohk @KtorZ