Skip to content

Conversation

@BiteTheDDDDt
Copy link
Contributor

What problem does this PR solve?

Currently, the number of our wait_channel_timers corresponds to the number of channels. After the modifications in this PR, the number of wait_channel_timers will align with the number of local channels, thereby reducing unnecessary timer generation.

This pull request refactors and improves code readability and safety in the exchange_sink_operator.cpp file by modernizing loop constructs, improving memory management, and enhancing error handling. The changes mainly focus on using range-based loops, smart pointers, and clearer logic for managing channel dependencies and timers.

Code modernization and safety improvements:

  • Replaced traditional for-loops with range-based for-loops for iterating over channels, improving readability and reducing indexing errors in multiple methods (init, open). [1] [2]
  • Updated memory management for _writer by switching from manual reset with new to std::make_unique, ensuring exception safety and clarity.
  • Used contains method instead of find for map lookups in _create_channels, making the code more concise and modern.
  • Changed range-based loop in the constructor to use const auto& for safety and clarity.

Dependency and error handling improvements:

  • Added a null pointer check for local channel dependencies and improved timer management, ensuring that dependencies are valid and timers are only added for non-null dependencies.

Check List (For Author)

  • Test

    • Regression test
    • Unit Test
    • Manual test (add detailed scripts or steps below)
    • No need to test or manual test. Explain why:
      • This is a refactor/code format and no logic has been changed.
      • Previous test can cover this change.
      • No code files have been changed.
      • Other reason
  • Behavior changed:

    • No.
    • Yes.
  • Does this need documentation?

    • No.
    • Yes.

Check List (For Reviewer who merge this PR)

  • Confirm the release note
  • Confirm test cases
  • Confirm document
  • Add branch pick label

@Thearas
Copy link
Contributor

Thearas commented Nov 4, 2025

Thank you for your contribution to Apache Doris.
Don't know what should be done next? See How to process your PR.

Please clearly describe your PR:

  1. What problem was fixed (it's best to include specific error reporting information). How it was fixed.
  2. Which behaviors were modified. What was the previous behavior, what is it now, why was it modified, and what possible impacts might there be.
  3. What features were added. Why was this function added?
  4. Which code was refactored and why was this part of the code refactored?
  5. Which functions were optimized and what is the difference before and after the optimization?

@BiteTheDDDDt
Copy link
Contributor Author

run buildall

@hello-stephen
Copy link
Contributor

BE UT Coverage Report

Increment line coverage 78.57% (11/14) 🎉

Increment coverage report
Complete coverage report

Category Coverage
Function Coverage 52.73% (18079/34287)
Line Coverage 38.05% (164452/432241)
Region Coverage 32.36% (125314/387250)
Branch Coverage 33.80% (54892/162387)

@hello-stephen
Copy link
Contributor

BE Regression && UT Coverage Report

Increment line coverage 85.71% (12/14) 🎉

Increment coverage report
Complete coverage report

Category Coverage
Function Coverage 71.49% (24095/33706)
Line Coverage 57.91% (250730/432986)
Region Coverage 52.82% (207496/392846)
Branch Coverage 54.67% (89388/163496)

@github-actions
Copy link
Contributor

github-actions bot commented Nov 4, 2025

PR approved by anyone and no changes requested.

@github-actions github-actions bot added the approved Indicates a PR has been approved by one committer. label Nov 5, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Nov 5, 2025

PR approved by at least one committer and no changes requested.

@BiteTheDDDDt BiteTheDDDDt merged commit 7a3ff66 into apache:master Nov 5, 2025
27 of 29 checks passed
wyxxxcat pushed a commit to wyxxxcat/doris that referenced this pull request Nov 18, 2025
Currently, the number of our wait_channel_timers corresponds to the
number of `channels`. After the modifications in this PR, the number of
wait_channel_timers will align with the number of `local channels`,
thereby reducing unnecessary timer generation.

This pull request refactors and improves code readability and safety in
the `exchange_sink_operator.cpp` file by modernizing loop constructs,
improving memory management, and enhancing error handling. The changes
mainly focus on using range-based loops, smart pointers, and clearer
logic for managing channel dependencies and timers.

**Code modernization and safety improvements:**

* Replaced traditional for-loops with range-based for-loops for
iterating over `channels`, improving readability and reducing indexing
errors in multiple methods (`init`, `open`).
[[1]](diffhunk://#diff-70038347289359e40966cf399f9a4165f6423f3634ceb13d496da4d4627ce073L85-R86)
[[2]](diffhunk://#diff-70038347289359e40966cf399f9a4165f6423f3634ceb13d496da4d4627ce073L217-R217)
* Updated memory management for `_writer` by switching from manual
`reset` with `new` to `std::make_unique`, ensuring exception safety and
clarity.
* Used `contains` method instead of `find` for map lookups in
`_create_channels`, making the code more concise and modern.
* Changed range-based loop in the constructor to use `const auto&` for
safety and clarity.

**Dependency and error handling improvements:**

* Added a null pointer check for local channel dependencies and improved
timer management, ensuring that dependencies are valid and timers are
only added for non-null dependencies.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by one committer. reviewed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants