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.

Sign up for GitHub

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

Jump to bottom

[20998] Record data in SQL #144

Open
wants to merge 52 commits into
base: main
Choose a base branch
from
Open

[20998] Record data in SQL #144

wants to merge 52 commits into from

Conversation

Tempate
Copy link
Contributor

@Tempate Tempate commented May 10, 2024 •
edited
Loading

Code improvements:

  • Make a Serializer to serialize and deserialize TopicQoS, TypeObjects, TypeIdentifiers, and DynamicTypes.
  • Make a time_utils to convert between different time formats.
  • Make the DdsRecorder and the DdsReplayer agnostic of the output format.
  • Refactor McapLogErrorTest to improve its readability.

Configuration improvements:

  • Move cleanup-period and max-pending-samples from specs to recorder.
  • Move safety-margin to resource-limits.
  • Configure safety-margin as a string (100MB, 128GiB, etc).

Bug fixes:

  • Avoid creating a payload when replaying a message in a topic without a reader.
  • Fix assert in ROS 2 topics.

@Tempate Tempate changed the base branch from main to stabilise/file_rotation May 10, 2024 09:10
@Tempate Tempate force-pushed the stabilise/file_rotation branch 2 times, most recently from ab73b8d to a9c882c Compare May 17, 2024 09:26
@Tempate Tempate changed the title Record data in SQL [20998] Record data in SQL Jun 5, 2024
@Tempate Tempate force-pushed the feature/sql branch 4 times, most recently from aca9425 to 8deea2e Compare June 13, 2024 10:50
Base automatically changed from stabilise/file_rotation to main June 17, 2024 05:35
LuciaEchevarria99 and others added 8 commits July 16, 2024 10:49
@LuciaEchevarria99
Change to FastDDS idl_serialize implementation
fc30c83 
Signed-off-by: Lucia Echevarria <[email protected]>
@Tempate
Move exceptions to a folder
5c92158 
Signed-off-by: tempate <[email protected]>
@Tempate
Strip serializing methods from the McapHandler
04fa0d1 
Signed-off-by: tempate <[email protected]>
@Tempate
Strip time utils from the McapHandler
82ceb00 
Signed-off-by: tempate <[email protected]>
@Tempate
Strip state transitions and buffer management from the McapHandler
c80db8f 
Signed-off-by: tempate <[email protected]>
@Tempate
Strip dynamic types management from the McapHandler
1904c67 
Signed-off-by: tempate <[email protected]>
@Tempate
Strip new sample generic processing from the McapHandler
29afbb5 
Signed-off-by: tempate <[email protected]>
@Tempate
Update includes in McapHandler and BaseHandler
10d3068 
Signed-off-by: tempate <[email protected]>
@Tempate
Rename McapReaderParticipantConfiguration to BaseReaderParticipantCon…
abd8d82 
…figuration

Signed-off-by: tempate <[email protected]>
@Tempate
Fix assert for ROS 2 topics
3dd2754 
Signed-off-by: tempate <[email protected]>
@Tempate
Move resource limits tests to MCAP
b7e27c9 
Signed-off-by: tempate <[email protected]>
@Tempate
FileCreationTests for SQL
62be8a9 
Signed-off-by: tempate <[email protected]>
@Tempate
Strip recording methods from McapFileCreationTest
78743f0 
Signed-off-by: tempate <[email protected]>
@Tempate
Only write dynamic types in SQL when closing the file
4e4446b 
Signed-off-by: tempate <[email protected]>
@Tempate
Store & read whether topics & types were formatted for ROS 2
60af701 
Signed-off-by: tempate <[email protected]>
@Tempate
LogErrorTest for SQL
b25c832 
Signed-off-by: tempate <[email protected]>
@Tempate
Fix McapLogErrorTest
aa37d10 
Signed-off-by: tempate <[email protected]>
@Tempate
Strip replaying methods from McapFileReadTest & McapFileReadWithTypeTest
f5768ab 
Signed-off-by: tempate <[email protected]>
@Tempate
FileReadTest for SQL
a7f41ce 
Signed-off-by: tempate <[email protected]>
@Tempate
Identify topics in SQL by the topic_name, type_name pair
7c787d2 
Signed-off-by: tempate <[email protected]>
@Tempate
Fix comments and logs incorrectly referring to MCAP
ae8737b 
Signed-off-by: tempate <[email protected]>
@Tempate
Identify messages by their writer GUID and sequence number
5d06eba 
Signed-off-by: tempate <[email protected]>
@Tempate
Rebase fix
babdf2f 
Signed-off-by: tempate <[email protected]>
@Tempate
Store a message's key in SQL with v3.0
350f322 
Signed-off-by: tempate <[email protected]>
@Tempate
Write SQL samples in bulk
e18bb16 
Signed-off-by: tempate <[email protected]>
@Tempate
Refactor Yaml configuration
7a45dd0 
Signed-off-by: tempate <[email protected]>
@Tempate
Fix the check when setting the current file size
7630fb5 
Signed-off-by: tempate <[email protected]>
@Tempate
Save data in CDR & JSON format
8ab4f8b 
Signed-off-by: tempate <[email protected]>
@Tempate
Refactor documentation
b6ae665 
Signed-off-by: tempate <[email protected]>
@Tempate
Rebase fix
b4277d6 
Signed-off-by: tempate <[email protected]>
@Tempate Tempate changed the base branch from main to integration/bump-3.0.x July 19, 2024 06:24
@Tempate
Tests for data-format
508b90b 
Signed-off-by: tempate <[email protected]>
LuciaEchevarria99
LuciaEchevarria99 requested changes Jul 23, 2024
View reviewed changes
ddsrecorder_participants/src/cpp/recorder/output/BaseWriter.cpp

// Open a new file to write the remaining data.
// Throw an exception if the file cannot be opened.
open_new_file_nts_(min_file_size + e.data_size_to_write());
Copy link
Contributor

Choose a reason for hiding this comment

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

This callback will be invoked during both MCAP and SQL recordings if disk is full. I believe it should have distinct implementations in McapWriter and SqlWriter. In the case of SQL, it is unnecessary to open a new file when the disk is full, as you can simply delete the beginning of the file.

ddsrecorder_participants/include/ddsrecorder_participants/recorder/message/SqlMessage.hpp
Comment on lines +66 to +67
void deserialize(
const fastdds::dds::DynamicType::_ref_type& dynamic_type);
Copy link
Contributor

Choose a reason for hiding this comment

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

This deserialize should be included in the Serializator class

Base automatically changed from integration/bump-3.0.x to main September 17, 2024 08:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@LuciaEchevarria99 LuciaEchevarria99 LuciaEchevarria99 requested changes

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Tempate @LuciaEchevarria99