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

DAC-289 Transaction payment by the Customer #46

Merged
merged 28 commits into from
Feb 9, 2023
Merged

DAC-289 Transaction payment by the Customer #46

merged 28 commits into from
Feb 9, 2023

Conversation

amnambiar
Copy link
Collaborator

Copy link
Collaborator

@bogdan-manole bogdan-manole left a comment

Choose a reason for hiding this comment

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

I will pause the review on this because i'm not able to start the app anymore ( I don't know if the problem was the same with some other previous approved PR's )
Practically what happens is:
if you type these startup commands:

rm -rf node_modules
npm install
npm start
  1. on my MacOS aarch64 computer the "Starting development server" hangs for around 1 minute and crashes with out of memory
  2. on my Linux x64 machine the "Starting development server" hangs around 3 minutes and crashes with the same error
Starting the development server...


<--- Last few GCs --->

[2951849:0x5c950d0]   251313 ms: Scavenge 4062.8 (4139.8) -> 4060.8 (4139.8) MB, 17.4 / 0.0 ms  (average mu = 0.887, current mu = 0.773) allocation failure
[2951849:0x5c950d0]   251406 ms: Scavenge 4063.5 (4139.8) -> 4062.0 (4141.3) MB, 12.8 / 0.0 ms  (average mu = 0.887, current mu = 0.773) allocation failure
[2951849:0x5c950d0]   259859 ms: Mark-sweep 4065.0 (4141.3) -> 4062.5 (4151.1) MB, 8352.6 / 0.0 ms  (average mu = 0.779, current mu = 0.059) allocation failure scavenge might not succeed


<--- JS stacktrace --->

FATAL ERROR: Reached heap limit Allocation failed - JavaScript heap out of memory
 1: 0xb02ec0 node::Abort() [/home/horus/.nvm/versions/node/v16.13.0/bin/node]
 2: 0xa181fb node::FatalError(char const*, char const*) [/home/horus/.nvm/versions/node/v16.13.0/bin/node]
 3: 0xced88e v8::Utils::ReportOOMFailure(v8::internal::Isolate*, char const*, bool) [/home/horus/.nvm/versions/node/v16.13.0/bin/node]
 4: 0xcedc07 v8::internal::V8::FatalProcessOutOfMemory(v8::internal::Isolate*, char const*, bool) [/home/horus/.nvm/versions/node/v16.13.0/bin/node]
 5: 0xea5ea5  [/home/horus/.nvm/versions/node/v16.13.0/bin/node]
 6: 0xeb557d v8::internal::Heap::CollectGarbage(v8::internal::AllocationSpace, v8::internal::GarbageCollectionReason, v8::GCCallbackFlags) [/home/horus/.nvm/versions/node/v16.13.0/bin/node]
 7: 0xeb827e v8::internal::Heap::AllocateRawWithRetryOrFailSlowPath(int, v8::internal::AllocationType, v8::internal::AllocationOrigin, v8::internal::AllocationAlignment) [/home/horus/.nvm/versions/node/v16.13.0/bin/node]
 8: 0xe796aa v8::internal::Factory::NewFillerObject(int, bool, v8::internal::AllocationType, v8::internal::AllocationOrigin) [/home/horus/.nvm/versions/node/v16.13.0/bin/node]
 9: 0x11f2e86 v8::internal::Runtime_AllocateInYoungGeneration(int, unsigned long*, v8::internal::Isolate*) [/home/horus/.nvm/versions/node/v16.13.0/bin/node]
10: 0x15e7879  [/home/horus/.nvm/versions/node/v16.13.0/bin/node]

@amnambiar
Copy link
Collaborator Author

FATAL ERROR: Reached heap limit Allocation failed - JavaScript heap out of memory
It worked for me after those steps. I will look into fixing any memory leaks or how to fix those in react , either with the current ticket in hand or in a newer one

@bogdan-manole
Copy link
Collaborator

FATAL ERROR: Reached heap limit Allocation failed - JavaScript heap out of memory
It worked for me after those steps. I will look into fixing any memory leaks or how to fix those in react , either with the current ticket in hand or in a newer one

a new ticket won't solve the fact that at least in my case this branch doesn't seem to work anymore ( btw, I tried #47 and it works )

@amnambiar
Copy link
Collaborator Author

Let me look

@amnambiar
Copy link
Collaborator Author

@bogdan-manole Try this for your global node.js, increasing the memory allocation, and it worked for me. Setting it to 4096 wasn't sufficient. We can embed it to the build scripts, maybe we can do that after the initial deployment trials if at all this error comes up.

export NODE_OPTIONS=--max_old_space_size=8048

@bogdan-manole
Copy link
Collaborator

@bogdan-manole Try this for your global node.js, increasing the memory allocation, and it worked for me. Setting it to 4096 wasn't sufficient. We can embed it to the build scripts, maybe we can do that after the initial deployment trials if at all this error comes up.

export NODE_OPTIONS=--max_old_space_size=8048

@bogdan-manole Try this for your global node.js, increasing the memory allocation, and it worked for me. Setting it to 4096 wasn't sufficient. We can embed it to the build scripts, maybe we can do that after the initial deployment trials if at all this error comes up.

export NODE_OPTIONS=--max_old_space_size=8048

I tried as you said, but the process of building it's still crashing when it reaches a total RAM > 5 GB

What worries me more it's why would we need such an increase in memory in one PR. In the master, a development node would reach a maximum of 512 MB, and now we need an increase of more than 10 times?!
Also, in master the server it starts in < 10 seconds, and now it takes around one minute, just to crash ( eating a lot of RAM)

I suspect we now have a library or some code change that makes that, and we rather know what it is and decide if it pays off for such a huge increase of resources

@amnambiar
Copy link
Collaborator Author

Agree, the added packages in this PR are the two essentials @emurgo/cardano-serialization-lib-asmjs and buffer

@amnambiar
Copy link
Collaborator Author

I've opened an issue Emurgo/cardano-serialization-lib#586 within the repository. I've also added in a sample repository with just the transaction bit in place. Let's wait until the reason behind this build error is tracked.

@amnambiar
Copy link
Collaborator Author

@bogdan-manole ,

A fix for the build error is in place. See this commit. Please run the below in the order, to ensure this fix (within react-web/):

$ git checkout DAC-289
$ git fetch origin
$ git pull origin DAC-289
$ rm -rf node_modules
$ npm install
$ npm run start

Please see if this works at your end as well.

Cc: @RSoulatIOHK

@amnambiar amnambiar merged commit aa9bb93 into master Feb 9, 2023
@amnambiar amnambiar deleted the DAC-289 branch February 9, 2023 16:50
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.

2 participants