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

fix: Performance problems #419

Merged
merged 8 commits into from
Jan 30, 2023
Merged

Conversation

antoinegelloz
Copy link
Contributor

@antoinegelloz antoinegelloz commented Jan 27, 2023

  • fixed a bug where a new logger was created at each request, replacing the previous one with wrapped version, creating a exponential performance problem.
  • fine tuned the Numscript cache system; The cost of each Numscript stored in cache is now calculated in bytes; The config can be modified with new flags.
  • improved TxsToScriptData method predictability; Postings now always generate the same Numscript (variables declarations list was not always sorted)
  • telemetry improvements

@antoinegelloz antoinegelloz requested a review from a team January 27, 2023 11:20
@codecov
Copy link

codecov bot commented Jan 27, 2023

Codecov Report

Merging #419 (f1ba45d) into main (cdd925b) will increase coverage by 0.25%.
The diff coverage is 83.73%.

@@            Coverage Diff             @@
##             main     #419      +/-   ##
==========================================
+ Coverage   68.72%   68.98%   +0.25%     
==========================================
  Files          94       95       +1     
  Lines        6511     6522      +11     
==========================================
+ Hits         4475     4499      +24     
+ Misses       1651     1638      -13     
  Partials      385      385              
Impacted Files Coverage Δ
cmd/server_start.go 73.68% <ø> (+7.64%) ⬆️
pkg/ledger/ledger.go 70.43% <ø> (-0.26%) ⬇️
pkg/storage/sqlstorage/logs.go 75.15% <ø> (-0.15%) ⬇️
pkg/storage/sqlstorage/transactions.go 81.36% <ø> (-0.09%) ⬇️
cmd/container.go 50.00% <55.55%> (-0.91%) ⬇️
pkg/api/middlewares/transaction.go 77.77% <72.72%> (+2.26%) ⬆️
pkg/ledger/cache.go 80.00% <80.00%> (ø)
pkg/ledger/executor.go 68.52% <80.64%> (+0.48%) ⬆️
pkg/storage/sqlstorage/driver.go 67.48% <82.60%> (-0.59%) ⬇️
pkg/api/middlewares/ledger_middleware.go 68.00% <83.33%> (-13.58%) ⬇️
... and 12 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

gfyrag and others added 4 commits January 27, 2023 14:39
* feat: add traces

* test: remove logging

* fix: remove logger context id key

* fix: comment tests

* test: Remove logs for cache

* feat: disable sql traces

* feat: commit transactions before write responses

* fix: request id in logs and set sql traces only in debug mode

* feat: clean spans

---------

Co-authored-by: Ragot Geoffrey <geoffrey.ragot@gmail.com>
@antoinegelloz antoinegelloz changed the title fix: TxsToScriptData improved predictability fix: Performance problems Jan 30, 2023
gfyrag
gfyrag previously approved these changes Jan 30, 2023
@flemzord flemzord merged commit 2e108d8 into main Jan 30, 2023
@flemzord flemzord deleted the fix/postings-to-numscript-predictability branch January 30, 2023 13:32
flemzord added a commit that referenced this pull request Jan 30, 2023
* chore: first

* fix: cleanup

* fix: improve cache config

* feat: add some traces (#420)

* fix: improve cache config

* feat: add traces (#422)

* feat: add traces

* test: remove logging

* fix: remove logger context id key

* fix: comment tests

* test: Remove logs for cache

* feat: disable sql traces

* feat: commit transactions before write responses

* fix: request id in logs and set sql traces only in debug mode

* feat: clean spans

---------

Co-authored-by: Ragot Geoffrey <geoffrey.ragot@gmail.com>

* fix: cleanup

* chore: numscript cache testing

---------

Co-authored-by: Ragot Geoffrey <geoffrey.ragot@gmail.com>
Co-authored-by: Maxence Maireaux <maxence@maireaux.fr>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants