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

fix the bug that the flight bytes are calculated incorrectly #9411

Merged
merged 5 commits into from
Aug 18, 2020

Conversation

vzqzhang
Copy link
Contributor

@vzqzhang vzqzhang commented Aug 18, 2020

Change Description

There is an option for nodeos that specifies the max flight bytes. However, it did not work fine since the calculation of the bytes in flight was wrong. The bytes in flight was always 0 or 1 so it never exceeds the max flight bytes.

It causes an issue that nodeos continues to accept new transactions when there is no new blocks until the process reaches the maximum file descriptors. It causes the nodeos malfunctioning and cannot be recovered. In some environments it causes the whole OS malfunctioning.

Change Type

Select ONE

  • Documentation
  • Stability bug fix
  • Other
  • Other - special case

Consensus Changes

  • Consensus Changes

API Changes

  • API Changes

Documentation Additions

  • Documentation Additions

@@ -448,7 +449,7 @@ namespace eosio {
*/
template<typename T>
static auto make_in_flight(T&& object, http_plugin_impl& impl) {
return in_flight<T>(std::forward<T>(object), impl);
return std::make_shared<in_flight<T>>(in_flight<T>(std::forward<T>(object), impl));
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this have to be a shared_ptr? Can't it just be moved into wrapped_then then when wrapped_then is destructed it will as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Does this have to be a shared_ptr? Can't it just be moved into wrapped_then then when wrapped_then is destructed it will as well.

No. Since in_flight disables its copy constructor, tracked_b gets deleted if it's not wrapped by a shared pointer.

Copy link
Contributor

Choose a reason for hiding this comment

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

Move it, not copy it.

Copy link
Contributor Author

@vzqzhang vzqzhang Aug 18, 2020

Choose a reason for hiding this comment

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

Move it, not copy it.

It failed to move. The error showed that it called the copy constructor, not the operator =.

Copy link
Contributor

Choose a reason for hiding this comment

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

there are a few reasons why we moved this to a std::shared_ptr the first is that it makes it copyable which is necessary for use with std::function. The second is that we have multiple asynchronous paths so there was no single lifecycle for tracked_b. We are leaning on the std::shared_ptr 's reference counting to give us the lifetime guarantees we want.

@vzqzhang vzqzhang merged commit 4ca3aae into develop Aug 18, 2020
@vzqzhang vzqzhang deleted the flight_bytes_bug_fix branch August 18, 2020 21:39
@gleehokie gleehokie changed the title fix the bug that the flight bytes are cacculated incorrect fix the bug that the flight bytes are calculated incorrectly Aug 20, 2020
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.

3 participants