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

[18966] SHM sending improvements #3642

Merged
merged 7 commits into from
Jul 13, 2023
Merged

Conversation

jsan-rt
Copy link
Contributor

@jsan-rt jsan-rt commented Jun 29, 2023

Description

Improve reliability of sending data through Shared Memory by detecting and deleting stale ports and remapping them when needed. It also adds a retry when sending data in the event the port being used is found to be obsolete.

Contributor Checklist

  • Commit messages follow the project guidelines.
  • The code follows the style guidelines of this project.
  • Tests that thoroughly check the new feature have been added/Regression tests checking the bug and its fix have been added; the added tests pass locally
  • Any new/modified methods have been properly documented using Doxygen.
  • Changes are ABI compatible.
  • Changes are API compatible.
  • [NA] New feature has been added to the versions.md file (if applicable).
  • [NA] New feature has been documented/Current behavior is correctly described in the documentation.
  • Applicable backports have been included in the description.

Reviewer Checklist

  • The PR has a milestone assigned.
  • Check contributor checklist is correct.
  • Check CI results: changes do not issue any warning.
  • Check CI results: failing tests are unrelated with the changes.

@EduPonz EduPonz added this to the v2.11.1 milestone Jul 6, 2023
@jsan-rt
Copy link
Contributor Author

jsan-rt commented Jul 6, 2023

@richiprosima please test this

@jsan-rt jsan-rt added the ci-pending PR which CI is running label Jul 6, 2023
@jsan-rt
Copy link
Contributor Author

jsan-rt commented Jul 6, 2023

@Mergifyio backport 2.6.x 2.9.x 2.10.x

@mergify
Copy link
Contributor

mergify bot commented Jul 6, 2023

backport 2.6.x 2.9.x 2.10.x

✅ Backports have been created

@jsan-rt
Copy link
Contributor Author

jsan-rt commented Jul 7, 2023

@richiprosima please test mac test windows

@jsan-rt
Copy link
Contributor Author

jsan-rt commented Jul 10, 2023

@richiprosima please test mac test windows

@JesusPoderoso JesusPoderoso added in progress Issue or PR which is being reviewed ci-pending PR which CI is running and removed ci-pending PR which CI is running in progress Issue or PR which is being reviewed labels Jul 11, 2023
@jsan-rt
Copy link
Contributor Author

jsan-rt commented Jul 11, 2023

@richiprosima please test this

MiguelCompany and others added 6 commits July 12, 2023 07:53
Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>
Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>
Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>
Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>
Signed-off-by: Javier Santiago <javiersantiago@eprosima.com>
Signed-off-by: Javier Santiago <javiersantiago@eprosima.com>
@jsan-rt jsan-rt force-pushed the bugfix/shm/sending_improvements branch from e7532d2 to d17af2d Compare July 12, 2023 05:53
@jsan-rt
Copy link
Contributor Author

jsan-rt commented Jul 12, 2023

@richiprosima please test this

Signed-off-by: Javier Santiago <javiersantiago@eprosima.com>
@jsan-rt
Copy link
Contributor Author

jsan-rt commented Jul 13, 2023

@richiprosima please test this

@jsan-rt
Copy link
Contributor Author

jsan-rt commented Jul 13, 2023

Address Sanitizer failure is unrelated.

Copy link
Contributor

@JLBuenoLopez JLBuenoLopez left a comment

Choose a reason for hiding this comment

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

LGTM with green Mac CI

@MiguelCompany
Copy link
Member

@richiprosima Please test mac

@JesusPoderoso JesusPoderoso added ready-to-merge Ready to be merged. CI and changes have been reviewed and approved. and removed ci-pending PR which CI is running labels Jul 13, 2023
@jsan-rt jsan-rt merged commit 2f80b06 into master Jul 13, 2023
@jsan-rt jsan-rt deleted the bugfix/shm/sending_improvements branch July 13, 2023 13:28
mergify bot pushed a commit that referenced this pull request Jul 13, 2023
* Refs #18966. Refactor of try_push.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs #18966. Retry try_push after regenerating output port.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs #18966. Clean all output ports on every send.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs #18966. Clean output ports when they have no listeners.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs #18966: Applied suggestions

Signed-off-by: Javier Santiago <javiersantiago@eprosima.com>

* Refs #18966: Uncrustify

Signed-off-by: Javier Santiago <javiersantiago@eprosima.com>

* Refs #18966: Prevent port cleanup on Windows platforms

Signed-off-by: Javier Santiago <javiersantiago@eprosima.com>

---------

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>
Signed-off-by: Javier Santiago <javiersantiago@eprosima.com>
Co-authored-by: Miguel Company <MiguelCompany@eprosima.com>
(cherry picked from commit 2f80b06)

# Conflicts:
#	src/cpp/rtps/transport/shared_mem/SharedMemTransport.cpp
mergify bot pushed a commit that referenced this pull request Jul 13, 2023
* Refs #18966. Refactor of try_push.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs #18966. Retry try_push after regenerating output port.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs #18966. Clean all output ports on every send.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs #18966. Clean output ports when they have no listeners.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs #18966: Applied suggestions

Signed-off-by: Javier Santiago <javiersantiago@eprosima.com>

* Refs #18966: Uncrustify

Signed-off-by: Javier Santiago <javiersantiago@eprosima.com>

* Refs #18966: Prevent port cleanup on Windows platforms

Signed-off-by: Javier Santiago <javiersantiago@eprosima.com>

---------

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>
Signed-off-by: Javier Santiago <javiersantiago@eprosima.com>
Co-authored-by: Miguel Company <MiguelCompany@eprosima.com>
(cherry picked from commit 2f80b06)
mergify bot pushed a commit that referenced this pull request Jul 13, 2023
* Refs #18966. Refactor of try_push.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs #18966. Retry try_push after regenerating output port.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs #18966. Clean all output ports on every send.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs #18966. Clean output ports when they have no listeners.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs #18966: Applied suggestions

Signed-off-by: Javier Santiago <javiersantiago@eprosima.com>

* Refs #18966: Uncrustify

Signed-off-by: Javier Santiago <javiersantiago@eprosima.com>

* Refs #18966: Prevent port cleanup on Windows platforms

Signed-off-by: Javier Santiago <javiersantiago@eprosima.com>

---------

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>
Signed-off-by: Javier Santiago <javiersantiago@eprosima.com>
Co-authored-by: Miguel Company <MiguelCompany@eprosima.com>
(cherry picked from commit 2f80b06)
MiguelCompany pushed a commit that referenced this pull request Jul 21, 2023
* SHM sending improvements (#3642)

* Refs #18966. Refactor of try_push.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs #18966. Retry try_push after regenerating output port.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs #18966. Clean all output ports on every send.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs #18966. Clean output ports when they have no listeners.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs #18966: Applied suggestions

Signed-off-by: Javier Santiago <javiersantiago@eprosima.com>

* Refs #18966: Uncrustify

Signed-off-by: Javier Santiago <javiersantiago@eprosima.com>

* Refs #18966: Prevent port cleanup on Windows platforms

Signed-off-by: Javier Santiago <javiersantiago@eprosima.com>

---------

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>
Signed-off-by: Javier Santiago <javiersantiago@eprosima.com>
Co-authored-by: Miguel Company <MiguelCompany@eprosima.com>
(cherry picked from commit 2f80b06)

# Conflicts:
#	src/cpp/rtps/transport/shared_mem/SharedMemTransport.cpp

* Fixed conflicts

Signed-off-by: Javier Santiago <javiersantiago@eprosima.com>

---------

Signed-off-by: Javier Santiago <javiersantiago@eprosima.com>
Co-authored-by: jsantiago-eProsima <90755661+jsantiago-eProsima@users.noreply.github.com>
Co-authored-by: Javier Santiago <javiersantiago@eprosima.com>
MiguelCompany pushed a commit that referenced this pull request Aug 8, 2023
* Refs #18966. Refactor of try_push.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs #18966. Retry try_push after regenerating output port.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs #18966. Clean all output ports on every send.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs #18966. Clean output ports when they have no listeners.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs #18966: Applied suggestions

Signed-off-by: Javier Santiago <javiersantiago@eprosima.com>

* Refs #18966: Uncrustify

Signed-off-by: Javier Santiago <javiersantiago@eprosima.com>

* Refs #18966: Prevent port cleanup on Windows platforms

Signed-off-by: Javier Santiago <javiersantiago@eprosima.com>

---------

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>
Signed-off-by: Javier Santiago <javiersantiago@eprosima.com>
Co-authored-by: Miguel Company <MiguelCompany@eprosima.com>
(cherry picked from commit 2f80b06)

Co-authored-by: jsantiago-eProsima <90755661+jsantiago-eProsima@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
first-to-merge ready-to-merge Ready to be merged. CI and changes have been reviewed and approved.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants