Skip to content

Conversation

@doisyg
Copy link

@doisyg doisyg commented Dec 23, 2024

Draft to discuss ways of solving #1886

Naive solution which throws away the first message that is not transient local when the buffer is full, as long as the number of messages at the beginning of the buffer accounts for less than 10% of the total number of messages in the buffer.

That will not solve completely the /tf_static use case if other transient local messages are recorded and periodically published: at some point there will be more than 10% of transient local messages in the buffer, and the first ones (i.e /tf_static) will be removed.

Interested in debating and suggestions on how to solve this properly :

  • Different buffers for each transient local topic ? But then how to control their max size ?
  • A buffer dedicated to /tf_static? But then loss of generality.
  • Something else ?

poc
Signed-off-by: Guillaume Doisy <[email protected]>
@doisyg
Copy link
Author

doisyg commented Dec 23, 2024

(PR non functional for Rolling, I was originally targeting Iron, will fix)

@MichaelOrlov
Copy link
Contributor

MichaelOrlov commented Jan 10, 2025

@doisyg The transient local messages usually appears somewhere at the beginning of the recording like /tf_static.
It seems make sense to consider adding an option for repeating some number of the transient local messages for each new recording bag rather than throwing away some percentage of the messages.
I have formulated a design proposal in another task #1159 (comment). Please take it into consideration.

@tonynajjar
Copy link

tonynajjar commented Dec 15, 2025

@doisyg The transient local messages usually appears somewhere at the beginning of the recording like /tf_static. It seems make sense to consider adding an option for repeating some number of the transient local messages for each new recording bag rather than throwing away some percentage of the messages. I have formulated a design proposal in another task #1159 (comment). Please take it into consideration.

@MichaelOrlov are you sure that your proposal would also solve what this PR is trying to solve? The bug described in the ticket has nothing to do with splitting, you can reproduce by:

1- Starting a a bag in snapshot with a relatively low --max-cache-size. It should record tf_static along with other topics
2- Publish a transient local topic, like /tf_static
3- Publish enough so that the bag size exceeds --max-cache-size
4- Call the snapshot service
5- See that /tf_static topic is not in the bag

Do you have any proposal to fix or workaround that?

@MichaelOrlov
Copy link
Contributor

@tonynajjar As from my other comment #1159 (comment)
The additional CLI options would be

$ ros2 bag record -a --repeat-latched /map=1 /tf_static=5

For snapshot mode, the straightforward approach would be to keep "latched" messages in a separate buffer and dump it to the disk before the circular buffer with other messages. Trying to reduce the circular buffer on a fly when latched mesages arrives seems is non-trivial. The feature would be more consistent with another feature for snapshot #1844 to limit the bag file by duration rather than by bag size.

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