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

Use better processing timer for logging #5843

Merged
merged 3 commits into from
Jun 21, 2023
Merged

Conversation

benaadams
Copy link
Member

@benaadams benaadams commented Jun 20, 2023

Changes

  • Use non wrapping timer

Types of changes

What types of changes does your code introduce?

  • Bugfix (a non-breaking change that fixes an issue)
  • New feature (a non-breaking change that adds functionality)
  • Breaking change (a change that causes existing functionality not to work as expected)
  • Optimization
  • Refactoring
  • Documentation update
  • Build-related changes
  • Other: Description

Testing

Requires testing

  • Yes
  • No

If yes, did you write tests?

  • Yes
  • No

Copy link
Member

@LukaszRozmej LukaszRozmej left a comment

Choose a reason for hiding this comment

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

I don't get it why this is needed, doesn't nlog already handle this in a good way?

@benaadams benaadams changed the title Use better processing timer, offload loads to background thread Use better processing timer for logging Jun 21, 2023
@@ -45,27 +41,32 @@ public NLogLogger(string loggerName = null)

public void Info(string text)
{
_logger.Info(text);
if (IsInfo)
Copy link
Member

Choose a reason for hiding this comment

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

why those ifs are useful? Normally we use them before calling this method to avoid allocating a string.

Copy link
Member Author

Choose a reason for hiding this comment

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

Are there and strongly typed; simple bool check. Saves on dropping though into all the logging abstractions and interfaces to decide not to log if the caller wasn't guarded

@kamilchodola
Copy link
Contributor

Merging to include in 1.20.0 branch

@kamilchodola kamilchodola merged commit 5638e2f into master Jun 21, 2023
61 checks passed
@kamilchodola kamilchodola deleted the Processing-timer branch June 21, 2023 12:58
kamilchodola pushed a commit that referenced this pull request Jun 21, 2023
* Use different processing timer

* Use single queue

* Revert to simpler
kamilchodola added a commit that referenced this pull request Aug 1, 2023
* Add missing peer types to limits (#5838)

* Add missing peer types to limits

* Add Reth

* Flaky test

* Use better processing timer for logging (#5843)

* Use different processing timer

* Use single queue

* Revert to simpler

* Add Gnosis Shanghai hard-fork timestamp (#5848)

* Don't use DarkGray (#5849)

* Bump to 1.20.0-rc

* MaxDegreeOfParallelism defaults for full pruning (#5662)

* other defaults for full pruning?

* degreeOfParalleism

* small refactor

* fix build

* Fixing BatchedTrieVistior

* 25% of cores

* Update Pruning config

* add logger

* fix

* Perf/smoother peer discovery (#5846)

* Continuously connect

* Added another delay due to disconnect

* Increase useless peer timeout

* Simple rate limiter

* Integrate with peer manager

* Adjust some stats

* Check all peer, and uses pending variable

* Make logic clearer

* Minor cleanup

* Missed cancellation token

* Cancel setup outgoing peer connection if throttled

* Lockless ratelimiter

* Addressing comment

* Minor adjustments

* Whitespace

* Having trouble getting candidate at higher speed

* Make test more consistent

* Even more lenient

(cherry picked from commit 6ab0a3a)

* Update RocksDB package (#5883)

(cherry picked from commit 4e2bf0f)

* Update DotNetty feed

(cherry picked from commit 3e330ad)

* Update DotNetty package

(cherry picked from commit 47a92ee)

* add more gnosis bootnodes (#5910)

(cherry picked from commit b7086a9)

* Remove `v` of non-legacy tx signature from RPC response (#5927)

* minor fixes

* Update version to 1.20.0

* Bump to 1.20.1

* Restore `v` in tx signature for Geth compatibility (#5937)

* Restore DotNetty feed (#5976)

* Revert `nuget.config`

* Update DotNetty package

---------

Co-authored-by: Ben Adams <thundercat@illyriad.co.uk>
Co-authored-by: Ruben Buniatyan <rubo@users.noreply.github.com>
Co-authored-by: Kamil Chodoła <kamil@nethermind.io>
Co-authored-by: Marek Moraczyński <marekm2504@gmail.com>
Co-authored-by: Amirul Ashraf <asdacap@gmail.com>
Co-authored-by: Marcin Sobczak <marcindsobczak@gmail.com>
kamilchodola pushed a commit that referenced this pull request Nov 28, 2023
* Use different processing timer

* Use single queue

* Revert to simpler
kamilchodola pushed a commit that referenced this pull request Nov 28, 2023
* Use different processing timer

* Use single queue

* Revert to simpler
kamilchodola added a commit that referenced this pull request Dec 8, 2023
* Use better processing timer for logging (#5843)

* Use different processing timer

* Use single queue

* Revert to simpler

* Don't use DarkGray (#5849)

* Bump to 1.20.0-rc

* Update DotNetty feed

(cherry picked from commit 3e330ad)

* Update DotNetty package

(cherry picked from commit 47a92ee)

* minor fixes

* Update version to 1.20.0

* Bump to 1.20.1

* Add workflow to compare rpcs

Fix path

Run on push temporarily

Fix file path

Fix inputs and missing vars

All fields not required

Inherit secrets when calling run-a-single-node-from-branch

Add default for cl_client

Use base_tag generated by run-a-single-node

Download artifacts from other post-merge-smoke-tests

Remove the old download-artifact

Change the name of token secret

Remove usage of external action

Allow to specify smoke tests branch to use

Remove additional option to fit dispatch limit

fix: wrong inputs string format

Export run id

Fix rpc url

Fix debug

Add runner's ip to `allowed_ips`

Maximum 10 inputs is allowed in workflow_dispatch

Add allowed_ips to workflow_dispatch inputs

Try to join two ips

Print `,` conditionally

Update public-ip

Debug

Use VPN

Add wireguard.conf

Fix location

Checkout the repository before installing wireguard

Update wireguard.conf to match nethermind repo secrets

Update environment variables to match secrets

Install flood before wireguard

Use include in matrix to specify both name and address

Revert "Use include in matrix to specify both name and address"

This reverts commit 71caa0c.

* Trigger workflow?

* Fix run-a-single-node-from-branch.yml

* Update branch name

* Wait for the node to sync

* Add a separate job to wait for node sync

* Fix missing checkout

* Use validator mode and set Pruning to None

* Increase timeout of wait to sync to 10 hours

* Change step timeout as well

* Wait 5.5h before running `wait_for_node_to_sync`
For github hosted runners time limit for a job is 6 hours.
It is less than a node needs to sync.
Thus we wait 5.5 hours before checking the sync status of node.

* Fix needs

* Wait another 5.5h

* Fix indentation

* Remove sleep2

* Remove debug info

* Change approach to multi nodes

* Fix desc

* Fix branch name for trigger

* Add logs

* Add hardcoded value

* Fix matrix

* Change matrix approach

* Fix json creation

* Fix path

* Select branch

* Absolut path

* revert path

* static path

* Remove ref

* bump checkout

* add logs

* Add fethc depth

* change debug log

* More logs

* Change logs

* Change back to old version of run0single-node

* revert changes

* Change job

* Add matrix to path

* Adjust ref

* fix allowed_ips

* missing needs

* Fix trigger job

* Use better processing timer for logging (#5843)

* Use different processing timer

* Use single queue

* Revert to simpler

* Don't use DarkGray (#5849)

* Bump to 1.20.0-rc

* Update DotNetty feed

(cherry picked from commit 3e330ad)

* Update DotNetty package

(cherry picked from commit 47a92ee)

* minor fixes

* Update version to 1.20.0

* Bump to 1.20.1

* Add workflow to compare rpcs

Fix path

Run on push temporarily

Fix file path

Fix inputs and missing vars

All fields not required

Inherit secrets when calling run-a-single-node-from-branch

Add default for cl_client

Use base_tag generated by run-a-single-node

Download artifacts from other post-merge-smoke-tests

Remove the old download-artifact

Change the name of token secret

Remove usage of external action

Allow to specify smoke tests branch to use

Remove additional option to fit dispatch limit

fix: wrong inputs string format

Export run id

Fix rpc url

Fix debug

Add runner's ip to `allowed_ips`

Maximum 10 inputs is allowed in workflow_dispatch

Add allowed_ips to workflow_dispatch inputs

Try to join two ips

Print `,` conditionally

Update public-ip

Debug

Use VPN

Add wireguard.conf

Fix location

Checkout the repository before installing wireguard

Update wireguard.conf to match nethermind repo secrets

Update environment variables to match secrets

Install flood before wireguard

Use include in matrix to specify both name and address

Revert "Use include in matrix to specify both name and address"

This reverts commit 71caa0c.

* Trigger workflow?

* Fix run-a-single-node-from-branch.yml

* Update branch name

* Wait for the node to sync

* Add a separate job to wait for node sync

* Fix missing checkout

* Use validator mode and set Pruning to None

* Increase timeout of wait to sync to 10 hours

* Change step timeout as well

* Wait 5.5h before running `wait_for_node_to_sync`
For github hosted runners time limit for a job is 6 hours.
It is less than a node needs to sync.
Thus we wait 5.5 hours before checking the sync status of node.

* Fix needs

* Wait another 5.5h

* Fix indentation

* Remove sleep2

* Remove debug info

* Change approach to multi nodes

* Fix branch name for trigger

* Fix desc

* Add logs

* Add hardcoded value

* Fix matrix

* Change matrix approach

* Fix json creation

* Fix path

* Select branch

* Absolut path

* revert path

* static path

* Remove ref

* bump checkout

* add logs

* Add fethc depth

* change debug log

* More logs

* Change logs

* Change back to old version of run0single-node

* revert changes

* Change job

* Add matrix to path

* Adjust ref

* fix allowed_ips

* missing needs

* Fix trigger job

* Revert bad rebase changes

* Fixed workflow for testing

* Fix needs

* Fix outputs

* add missing $

* Add more logs

* Change rpc_urls

* Change approach of node creation

* Comment out

* Fix needs

* Change a way of applying artifacts and rpc_urls

* Adjust needs

* Adjust

* Add missing variables

* Fix invalid file read

* Fix temp infura value

* Fix to much  '

* Fix artifact save

* Fix branches fetching

* Temp remove needs

* Switch to first job needs

* Fix list fill

* Invalid path

* Add verbose logging

* Adjust

* add missing repo

* Rollback run_id

* Fix needs

* Trigger

* Fix needs

* Change apprioach of downloading artifacts

* Add more params to artifact download

* Download all from current run

* Change the runid

* Add search

* Fix typo

* Change way of downlaoding artifacts

* Test a different run-id

* Change approach

* Add mising custom id

* Change approach to only 1 branch compare

* Fix

* Fix2

* add if commented

* Fetch only most recent run for selected ref

* Change name of job

* Fix a ref_name displayed

* Fix ref name

* Cleanup

* Fix typo

* Clean refs

* Change artifacts download path

* Change action temporarily

* Add path param

* Switch to david action type

* Trigger new nodes

* Rollback needs

* Fix if check for docker build creation

* Rolback to classic artifacts downoad

* Add better docker image check

* Add missing if condition

* Fix paths and add temp sepolia

* Change delimiter

* Adjust waiting for node

* Fix wrong output

* Mock and test

* Fix issue

* Fix issue2

* Fix issue

* Remove deps

* Fix invalid "

* Revert comments back

* Test on existing infra

* Fix bad replace

* Change aproach of running

* Fix failure

* Add a better order of artifacts on a rpc_list

* Adjust

* Fix

* Rollback to env

* Adjust flood script

* Fix rpc_urls creation

* Hardcode urls for test

* Comment out needs

* Add missing equality tests

* Change to kch flood fork

* Rollback to official flood

* Generate new nodes

* Add misiing extension

* adjist script and add logs

* Fix typo

* Test

* Change approach slightly

* One mroe adjustment

* Adjust to mainnet and add missing envs

* Bump timeout to something higher to fit tests

* Clean Up

* Add missing changes

---------

Co-authored-by: Ben Adams <thundercat@illyriad.co.uk>
Co-authored-by: Kamil Chodoła <kamil@nethermind.io>
Co-authored-by: Ruben Buniatyan <rubo@users.noreply.github.com>
Co-authored-by: Marcin Sobczak <marcindsobczak@gmail.com>
Co-authored-by: Piotr Piwoński <piwonskp>
Co-authored-by: Piotr Piwoński <piwonskp@gmail.com>
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

3 participants