Skip to content

perf(event bus): Remove expensive Logger debug call in PublishEventTx#2911

Merged
andynog merged 2 commits intocometbft:mainfrom
osmosis-labs:dev/remove_expensive_logger_with_call
Apr 29, 2024
Merged

perf(event bus): Remove expensive Logger debug call in PublishEventTx#2911
andynog merged 2 commits intocometbft:mainfrom
osmosis-labs:dev/remove_expensive_logger_with_call

Conversation

@ValarDragon
Copy link
Contributor

@ValarDragon ValarDragon commented Apr 27, 2024

Component of #2869

This on its own is an expected 1% speedup to blocksync on osmosis mainnet right now.

I originally considered keeping the log lines but with only creating the logger cost if there is an error, but these are debug logs I've never seen. I think its better to just remove these debug logs directly, rather than worry about maintaining them. These aren't even that concerning scenarios I feel like, as more of the stack moves away from these.


PR checklist

  • Tests written/updated - Covered by existing tests
  • Changelog entry added in .changelog (we use unclog to manage our changelog)
  • Updated relevant documentation (docs/ or spec/) and code comments
  • Title follows the Conventional Commits spec

@ValarDragon ValarDragon requested a review from a team as a code owner April 27, 2024 17:42
@ValarDragon ValarDragon requested a review from a team April 27, 2024 17:42
@ValarDragon ValarDragon changed the title perf: Remove expensive Logger debug call in PublishEventTx perf(event bus): Remove expensive Logger debug call in PublishEventTx Apr 27, 2024
Copy link
Collaborator

@melekes melekes left a comment

Choose a reason for hiding this comment

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

Thanks @ValarDragon ❤️

@melekes melekes added backport-to-v0.37.x backport-to-v0.38.x Tell Mergify to backport the PR to v0.38.x labels Apr 29, 2024
@andynog andynog self-assigned this Apr 29, 2024
Copy link
Collaborator

@andynog andynog left a comment

Choose a reason for hiding this comment

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

Looks good to me, I don't see any value on logging these messages and seems that removing them will only benefit performance. Thanks @ValarDragon

@andynog andynog added this pull request to the merge queue Apr 29, 2024
Merged via the queue into cometbft:main with commit ce68e90 Apr 29, 2024
mergify bot pushed a commit that referenced this pull request Apr 29, 2024
…#2911)

Component of #2869

This on its own is an expected 1% speedup to blocksync on osmosis
mainnet right now.

I originally considered keeping the log lines but with only creating the
logger cost if there is an error, but these are debug logs I've never
seen. I think its better to just remove these debug logs directly,
rather than worry about maintaining them. These aren't even that
concerning scenarios I feel like, as more of the stack moves away from
these.

---

#### PR checklist

- [x] Tests written/updated - Covered by existing tests
- [x] Changelog entry added in `.changelog` (we use
[unclog](https://github.com/informalsystems/unclog) to manage our
changelog)
- [X] Updated relevant documentation (`docs/` or `spec/`) and code
comments
- [X] Title follows the [Conventional
Commits](https://www.conventionalcommits.org/en/v1.0.0/) spec

(cherry picked from commit ce68e90)

# Conflicts:
#	types/event_bus.go
mergify bot pushed a commit that referenced this pull request Apr 29, 2024
…#2911)

Component of #2869

This on its own is an expected 1% speedup to blocksync on osmosis
mainnet right now.

I originally considered keeping the log lines but with only creating the
logger cost if there is an error, but these are debug logs I've never
seen. I think its better to just remove these debug logs directly,
rather than worry about maintaining them. These aren't even that
concerning scenarios I feel like, as more of the stack moves away from
these.

---

#### PR checklist

- [x] Tests written/updated - Covered by existing tests
- [x] Changelog entry added in `.changelog` (we use
[unclog](https://github.com/informalsystems/unclog) to manage our
changelog)
- [X] Updated relevant documentation (`docs/` or `spec/`) and code
comments
- [X] Title follows the [Conventional
Commits](https://www.conventionalcommits.org/en/v1.0.0/) spec

(cherry picked from commit ce68e90)
mergify bot pushed a commit that referenced this pull request Apr 29, 2024
…#2911)

Component of #2869

This on its own is an expected 1% speedup to blocksync on osmosis
mainnet right now.

I originally considered keeping the log lines but with only creating the
logger cost if there is an error, but these are debug logs I've never
seen. I think its better to just remove these debug logs directly,
rather than worry about maintaining them. These aren't even that
concerning scenarios I feel like, as more of the stack moves away from
these.

---

#### PR checklist

- [x] Tests written/updated - Covered by existing tests
- [x] Changelog entry added in `.changelog` (we use
[unclog](https://github.com/informalsystems/unclog) to manage our
changelog)
- [X] Updated relevant documentation (`docs/` or `spec/`) and code
comments
- [X] Title follows the [Conventional
Commits](https://www.conventionalcommits.org/en/v1.0.0/) spec

(cherry picked from commit ce68e90)

# Conflicts:
#	types/event_bus.go
andynog pushed a commit that referenced this pull request Apr 29, 2024
… (backport #2911) (#2935)

Component of #2869 

This on its own is an expected 1% speedup to blocksync on osmosis
mainnet right now.

I originally considered keeping the log lines but with only creating the
logger cost if there is an error, but these are debug logs I've never
seen. I think its better to just remove these debug logs directly,
rather than worry about maintaining them. These aren't even that
concerning scenarios I feel like, as more of the stack moves away from
these.


---

#### PR checklist

- [x] Tests written/updated - Covered by existing tests
- [x] Changelog entry added in `.changelog` (we use
[unclog](https://github.com/informalsystems/unclog) to manage our
changelog)
- [X] Updated relevant documentation (`docs/` or `spec/`) and code
comments
- [X] Title follows the [Conventional
Commits](https://www.conventionalcommits.org/en/v1.0.0/) spec
<hr>This is an automatic backport of pull request #2911 done by
[Mergify](https://mergify.com).

Co-authored-by: Dev Ojha <ValarDragon@users.noreply.github.com>
andynog added a commit that referenced this pull request Apr 29, 2024
… (backport #2911) (#2936)

Component of #2869 

This on its own is an expected 1% speedup to blocksync on osmosis
mainnet right now.

I originally considered keeping the log lines but with only creating the
logger cost if there is an error, but these are debug logs I've never
seen. I think its better to just remove these debug logs directly,
rather than worry about maintaining them. These aren't even that
concerning scenarios I feel like, as more of the stack moves away from
these.


---

#### PR checklist

- [x] Tests written/updated - Covered by existing tests
- [x] Changelog entry added in `.changelog` (we use
[unclog](https://github.com/informalsystems/unclog) to manage our
changelog)
- [X] Updated relevant documentation (`docs/` or `spec/`) and code
comments
- [X] Title follows the [Conventional
Commits](https://www.conventionalcommits.org/en/v1.0.0/) spec
<hr>This is an automatic backport of pull request #2911 done by
[Mergify](https://mergify.com).

---------

Co-authored-by: Dev Ojha <ValarDragon@users.noreply.github.com>
Co-authored-by: Andy Nogueira <me@andynogueira.dev>
andynog added a commit that referenced this pull request Apr 29, 2024
… (backport #2911) (#2937)

Component of #2869 

This on its own is an expected 1% speedup to blocksync on osmosis
mainnet right now.

I originally considered keeping the log lines but with only creating the
logger cost if there is an error, but these are debug logs I've never
seen. I think its better to just remove these debug logs directly,
rather than worry about maintaining them. These aren't even that
concerning scenarios I feel like, as more of the stack moves away from
these.


---

#### PR checklist

- [x] Tests written/updated - Covered by existing tests
- [x] Changelog entry added in `.changelog` (we use
[unclog](https://github.com/informalsystems/unclog) to manage our
changelog)
- [X] Updated relevant documentation (`docs/` or `spec/`) and code
comments
- [X] Title follows the [Conventional
Commits](https://www.conventionalcommits.org/en/v1.0.0/) spec
<hr>This is an automatic backport of pull request #2911 done by
[Mergify](https://mergify.com).

---------

Co-authored-by: Dev Ojha <ValarDragon@users.noreply.github.com>
Co-authored-by: Andy Nogueira <me@andynogueira.dev>
@adizere adizere mentioned this pull request May 1, 2024
2 tasks
czarcas7ic pushed a commit to osmosis-labs/cometbft that referenced this pull request May 2, 2024
… (backport cometbft#2911) (cometbft#2937)

Component of cometbft#2869 

This on its own is an expected 1% speedup to blocksync on osmosis
mainnet right now.

I originally considered keeping the log lines but with only creating the
logger cost if there is an error, but these are debug logs I've never
seen. I think its better to just remove these debug logs directly,
rather than worry about maintaining them. These aren't even that
concerning scenarios I feel like, as more of the stack moves away from
these.


---

#### PR checklist

- [x] Tests written/updated - Covered by existing tests
- [x] Changelog entry added in `.changelog` (we use
[unclog](https://github.com/informalsystems/unclog) to manage our
changelog)
- [X] Updated relevant documentation (`docs/` or `spec/`) and code
comments
- [X] Title follows the [Conventional
Commits](https://www.conventionalcommits.org/en/v1.0.0/) spec
<hr>This is an automatic backport of pull request cometbft#2911 done by
[Mergify](https://mergify.com).

---------

Co-authored-by: Dev Ojha <ValarDragon@users.noreply.github.com>
Co-authored-by: Andy Nogueira <me@andynogueira.dev>
czarcas7ic added a commit to osmosis-labs/cometbft that referenced this pull request May 2, 2024
… (backport cometbft#2911) (cometbft#2937) (#38)

Component of cometbft#2869 

This on its own is an expected 1% speedup to blocksync on osmosis
mainnet right now.

I originally considered keeping the log lines but with only creating the
logger cost if there is an error, but these are debug logs I've never
seen. I think its better to just remove these debug logs directly,
rather than worry about maintaining them. These aren't even that
concerning scenarios I feel like, as more of the stack moves away from
these.


---

#### PR checklist

- [x] Tests written/updated - Covered by existing tests
- [x] Changelog entry added in `.changelog` (we use
[unclog](https://github.com/informalsystems/unclog) to manage our
changelog)
- [X] Updated relevant documentation (`docs/` or `spec/`) and code
comments
- [X] Title follows the [Conventional
Commits](https://www.conventionalcommits.org/en/v1.0.0/) spec
<hr>This is an automatic backport of pull request cometbft#2911 done by
[Mergify](https://mergify.com).

---------

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Co-authored-by: Dev Ojha <ValarDragon@users.noreply.github.com>
Co-authored-by: Andy Nogueira <me@andynogueira.dev>
mergify bot added a commit to osmosis-labs/cometbft that referenced this pull request May 2, 2024
… (backport cometbft#2911) (cometbft#2937) (#38)

Component of cometbft#2869

This on its own is an expected 1% speedup to blocksync on osmosis
mainnet right now.

I originally considered keeping the log lines but with only creating the
logger cost if there is an error, but these are debug logs I've never
seen. I think its better to just remove these debug logs directly,
rather than worry about maintaining them. These aren't even that
concerning scenarios I feel like, as more of the stack moves away from
these.

---

#### PR checklist

- [x] Tests written/updated - Covered by existing tests
- [x] Changelog entry added in `.changelog` (we use
[unclog](https://github.com/informalsystems/unclog) to manage our
changelog)
- [X] Updated relevant documentation (`docs/` or `spec/`) and code
comments
- [X] Title follows the [Conventional
Commits](https://www.conventionalcommits.org/en/v1.0.0/) spec
<hr>This is an automatic backport of pull request cometbft#2911 done by
[Mergify](https://mergify.com).

---------

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Co-authored-by: Dev Ojha <ValarDragon@users.noreply.github.com>
Co-authored-by: Andy Nogueira <me@andynogueira.dev>
(cherry picked from commit a9991fd)
mergify bot added a commit to osmosis-labs/cometbft that referenced this pull request May 2, 2024
… (backport cometbft#2911) (cometbft#2937) (#38)

Component of cometbft#2869

This on its own is an expected 1% speedup to blocksync on osmosis
mainnet right now.

I originally considered keeping the log lines but with only creating the
logger cost if there is an error, but these are debug logs I've never
seen. I think its better to just remove these debug logs directly,
rather than worry about maintaining them. These aren't even that
concerning scenarios I feel like, as more of the stack moves away from
these.

---

#### PR checklist

- [x] Tests written/updated - Covered by existing tests
- [x] Changelog entry added in `.changelog` (we use
[unclog](https://github.com/informalsystems/unclog) to manage our
changelog)
- [X] Updated relevant documentation (`docs/` or `spec/`) and code
comments
- [X] Title follows the [Conventional
Commits](https://www.conventionalcommits.org/en/v1.0.0/) spec
<hr>This is an automatic backport of pull request cometbft#2911 done by
[Mergify](https://mergify.com).

---------

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Co-authored-by: Dev Ojha <ValarDragon@users.noreply.github.com>
Co-authored-by: Andy Nogueira <me@andynogueira.dev>
(cherry picked from commit a9991fd)
czarcas7ic added a commit to osmosis-labs/cometbft that referenced this pull request May 2, 2024
… (backport cometbft#2911) (cometbft#2937) (#38) (#44)

Component of cometbft#2869

This on its own is an expected 1% speedup to blocksync on osmosis
mainnet right now.

I originally considered keeping the log lines but with only creating the
logger cost if there is an error, but these are debug logs I've never
seen. I think its better to just remove these debug logs directly,
rather than worry about maintaining them. These aren't even that
concerning scenarios I feel like, as more of the stack moves away from
these.

---

#### PR checklist

- [x] Tests written/updated - Covered by existing tests
- [x] Changelog entry added in `.changelog` (we use
[unclog](https://github.com/informalsystems/unclog) to manage our
changelog)
- [X] Updated relevant documentation (`docs/` or `spec/`) and code
comments
- [X] Title follows the [Conventional
Commits](https://www.conventionalcommits.org/en/v1.0.0/) spec
<hr>This is an automatic backport of pull request cometbft#2911 done by
[Mergify](https://mergify.com).

---------

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Co-authored-by: Dev Ojha <ValarDragon@users.noreply.github.com>
Co-authored-by: Andy Nogueira <me@andynogueira.dev>
(cherry picked from commit a9991fd)

Co-authored-by: Adam Tucker <adam@osmosis.team>
czarcas7ic added a commit to osmosis-labs/cometbft that referenced this pull request May 2, 2024
… (backport cometbft#2911) (cometbft#2937) (#38) (#45)

Component of cometbft#2869

This on its own is an expected 1% speedup to blocksync on osmosis
mainnet right now.

I originally considered keeping the log lines but with only creating the
logger cost if there is an error, but these are debug logs I've never
seen. I think its better to just remove these debug logs directly,
rather than worry about maintaining them. These aren't even that
concerning scenarios I feel like, as more of the stack moves away from
these.

---

#### PR checklist

- [x] Tests written/updated - Covered by existing tests
- [x] Changelog entry added in `.changelog` (we use
[unclog](https://github.com/informalsystems/unclog) to manage our
changelog)
- [X] Updated relevant documentation (`docs/` or `spec/`) and code
comments
- [X] Title follows the [Conventional
Commits](https://www.conventionalcommits.org/en/v1.0.0/) spec
<hr>This is an automatic backport of pull request cometbft#2911 done by
[Mergify](https://mergify.com).

---------

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Co-authored-by: Dev Ojha <ValarDragon@users.noreply.github.com>
Co-authored-by: Andy Nogueira <me@andynogueira.dev>
(cherry picked from commit a9991fd)

Co-authored-by: Adam Tucker <adam@osmosis.team>
mattac21 pushed a commit that referenced this pull request Sep 9, 2025
…#2911)

Component of #2869 

This on its own is an expected 1% speedup to blocksync on osmosis
mainnet right now.

I originally considered keeping the log lines but with only creating the
logger cost if there is an error, but these are debug logs I've never
seen. I think its better to just remove these debug logs directly,
rather than worry about maintaining them. These aren't even that
concerning scenarios I feel like, as more of the stack moves away from
these.


---

#### PR checklist

- [x] Tests written/updated - Covered by existing tests
- [x] Changelog entry added in `.changelog` (we use
[unclog](https://github.com/informalsystems/unclog) to manage our
changelog)
- [X] Updated relevant documentation (`docs/` or `spec/`) and code
comments
- [X] Title follows the [Conventional
Commits](https://www.conventionalcommits.org/en/v1.0.0/) spec
mattac21 pushed a commit that referenced this pull request Sep 10, 2025
…#2911)

Component of #2869 

This on its own is an expected 1% speedup to blocksync on osmosis
mainnet right now.

I originally considered keeping the log lines but with only creating the
logger cost if there is an error, but these are debug logs I've never
seen. I think its better to just remove these debug logs directly,
rather than worry about maintaining them. These aren't even that
concerning scenarios I feel like, as more of the stack moves away from
these.


---

#### PR checklist

- [x] Tests written/updated - Covered by existing tests
- [x] Changelog entry added in `.changelog` (we use
[unclog](https://github.com/informalsystems/unclog) to manage our
changelog)
- [X] Updated relevant documentation (`docs/` or `spec/`) and code
comments
- [X] Title follows the [Conventional
Commits](https://www.conventionalcommits.org/en/v1.0.0/) spec
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport-to-v0.38.x Tell Mergify to backport the PR to v0.38.x

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants