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

Core/196/handle backlog deletes #337

Closed
wants to merge 12 commits into from

Conversation

rhsimplex
Copy link
Contributor

@rhsimplex rhsimplex commented May 27, 2016

Ready for review. Resolves #196.

@codecov-io
Copy link

codecov-io commented May 27, 2016

Current coverage is 91.08%

Merging #337 into master will increase coverage by 1.20%

@@             master       #337   diff @@
==========================================
  Files            18         18          
  Lines          1285       1357    +72   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits           1155       1236    +81   
+ Misses          130        121     -9   
  Partials          0          0          

Sunburst

Powered by Codecov. Last updated by db3e88e...941219c

@rhsimplex rhsimplex closed this May 30, 2016
@rhsimplex rhsimplex reopened this May 30, 2016
@r-marques r-marques mentioned this pull request May 30, 2016
17 tasks

if validity and list(validity.values()).count(Bigchain.BLOCK_VALID) == 1:
# tx made it into a block, and can safely be deleted
self.q_tx_delete.put(tx['id'])
Copy link
Contributor

Choose a reason for hiding this comment

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

why do we need to store the transactions here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As opposed to just the txid? No I don't think so, I will make the changes.

Copy link
Contributor

Choose a reason for hiding this comment

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

It was more what do we do with q_tx_delete?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's there because BacklogDeleteRevert is a derived class of block, so I didn't have to rewrite the validate_transactions. It expects a queue to take invalid transactions, but yes it's functionally useless.

Copy link
Contributor

Choose a reason for hiding this comment

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

It should not be a problem. You never start the delete_transactions process

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yikes, my mistake...but couldn't the queue fill up and start throwing exceptions?

@rhsimplex
Copy link
Contributor Author

Should I close this? Will backlog also be append only?

@r-marques
Copy link
Contributor

This is not that simple because the backlog is used as a temporary storage for unprocessed transactions

@vrde
Copy link
Contributor

vrde commented Sep 6, 2016

Stale PR, closing it.

@vrde vrde closed this Sep 6, 2016
@vrde vrde deleted the core/196/handle-backlog-deletes branch March 21, 2018 15:41
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.

None yet

5 participants