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 incosistences between Pub/Sub history and RTPS history [12620] #2239

Merged
merged 7 commits into from
Oct 27, 2021

Conversation

richiware
Copy link
Member

When it is received a HEARTBEAT, its firstSN is used to find fragmented samples not fully assembled yet and remove them. This functionality is done by ReaderHistory::remove_fragmented_changes_until. But this function only removes the changes from the ReaderHistory, but not from SubscriberHistory, provoking inconsistences in the instances' vector.

@richiware richiware changed the title Fix incosistences between Pub/Sub history and RTPS history Fix incosistences between Pub/Sub history and RTPS history [12620] Sep 29, 2021
Signed-off-by: Ricardo González <ricardo@richiware.dev>
Signed-off-by: Ricardo González <ricardo@richiware.dev>
Signed-off-by: Ricardo González <ricardo@richiware.dev>
@richiware richiware force-pushed the bugfix/inconsistences-between-histories branch from b6f7a48 to a622869 Compare October 4, 2021 09:41
Signed-off-by: Ricardo González <ricardo@richiware.dev>
Copy link
Contributor

@IkerLuengo IkerLuengo left a comment

Choose a reason for hiding this comment

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

Modifications seem OK, but there is a new warning on the MAC build:

SubscriberHistory.h:94 - 'completed_change' overrides a member function but is not marked 'override'

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

SubscriberHistory.h:94 - 'completed_change' overrides a member function but is not marked 'override'

@IkerLuengo I fixed this on e87d31a

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>
Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>
@MiguelCompany MiguelCompany merged commit 6003592 into master Oct 27, 2021
@MiguelCompany MiguelCompany deleted the bugfix/inconsistences-between-histories branch October 27, 2021 09:21
@MiguelCompany
Copy link
Member

@Mergifyio backport 2.3.x

mergify bot pushed a commit that referenced this pull request Oct 27, 2021
* Refs #12161. Add test large fragmented samples with key

Signed-off-by: Ricardo González <ricardo@richiware.dev>

* Refs #12161. Fix deserializing not completed sample

Signed-off-by: Ricardo González <ricardo@richiware.dev>

* Refs #12161. Fix inconsistence between histories

Signed-off-by: Ricardo González <ricardo@richiware.dev>

* Refs #12161. Fix uncrustify

Signed-off-by: Ricardo González <ricardo@richiware.dev>

* Refs 12620. Added override on overriden method.

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

* Added new virtual method to ReaderHistory mock

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

* Refs 12620. Uncrustify.

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

Co-authored-by: Miguel Company <miguelcompany@eprosima.com>
(cherry picked from commit 6003592)

# Conflicts:
#	test/blackbox/CMakeLists.txt
#	test/blackbox/common/BlackboxTestsPubSubFragments.cpp
@mergify
Copy link
Contributor

mergify bot commented Oct 27, 2021

backport 2.3.x

✅ Backports have been created

@richiware
Copy link
Member Author

👏🏾 Thanks @MiguelCompany

MiguelCompany pushed a commit that referenced this pull request Mar 18, 2022
* Fix incosistences between Pub/Sub history and RTPS history (#2239)

* Refs #12161. Add test large fragmented samples with key

Signed-off-by: Ricardo González <ricardo@richiware.dev>

* Refs #12161. Fix deserializing not completed sample

Signed-off-by: Ricardo González <ricardo@richiware.dev>

* Refs #12161. Fix inconsistence between histories

Signed-off-by: Ricardo González <ricardo@richiware.dev>

* Refs #12161. Fix uncrustify

Signed-off-by: Ricardo González <ricardo@richiware.dev>

* Refs 12620. Added override on overriden method.

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

* Added new virtual method to ReaderHistory mock

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

* Refs 12620. Uncrustify.

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

Co-authored-by: Miguel Company <miguelcompany@eprosima.com>
(cherry picked from commit 6003592)

# Conflicts:
#	test/blackbox/CMakeLists.txt
#	test/blackbox/common/BlackboxTestsPubSubFragments.cpp

* Refs #12620. Fix conflict

Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>

Co-authored-by: Ricardo González <ricardo@richiware.dev>
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.

3 participants