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

Delete the oldest entry when ringbuffer is full #579

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jianjun-huang
Copy link

The change will delete the oldest entries in the ringbuffer when the ringbuffer is full

@michael-methner
Copy link
Collaborator

Hello @jianjun-huang ,
there are reasons to not overwrite the first messages as well as there are reasons to have the ring buffer to be rotated.

Could you make this feature configurable via enviroment_variables (like the ringbuffer size already is)? The default behavior should be to keep the oldest logs. This enables to debug startup issues for example.

@minminlittleshrimp
Copy link
Collaborator

Hello @jianjun-huang
thanks for the idea.
However, as @michael-methner mentioned, we would not want our current mechanism changing, but refer a configurable feature of log rotating inside the ring buffer.
About the current mechanism, you could find more info in this disscussion: #533
Please kindly rework this patch, we will review when it done.
Thank you

@jianjun-huang
Copy link
Author

Hi @minminlittleshrimp
I'm trying to work on making the feature configurable.

Just like the discussion in #533, the same ringbuffer implementation is used in dlt-daemon and libdlt. It is easier to add a new configuration parameter in the dlt.conf to change the behavior on dlt-daemon site. But there is no good solution on the application side(libdlt). Environment variable is a potential application wide but not a system wide solution. The user may observe different ringbuffer behaviors among the applications and dlt-daemon.

Also, in the AUTOSAR_SWS_LogAndTrace_R22_11.pdf, it mentions:
"[SWS_LOG_00001] dLog message logged before the Logging framework is able
to process them (e.g. daemon communication is not established) shall be queued.
The queue size is defined by LogAndTraceInstantiation.queueSize. If this size
is exceeded the oldest entries shall be discarded.c(RS_LT_00003, RS_LT_00052)"

It seems that we should always remove the oldest entries when ringbuffer is full.

What are your thoughts on these questions?

@minminlittleshrimp
Copy link
Collaborator

Hello @jianjun-huang
Unlike LTTng, DLT tool in fact part of COMstack/Autosar, and hence, serving the purpose of logging and tracing mainly on Automotive side. DLT is designed and constructed under Autosar standard, so honestly if the doc forcing to discard new log, no ring buffer rotating, then this behavior will be and always be the default.
On my point of view, since dlt is OSS project, it can be enhanced the flexibility, reusability by adding new feature but still keeping the original/standard one. In this case, I agree with Michael to at least make the log rotating optional! Using env variable seems fine here, by setting a variable to make a rotation of log once at a time sounds good to me then.
Another suggestion is to look into the way logstorage buffer rotating with option overwrite behavior DISCARD_OLD.
Regards

@minminlittleshrimp
Copy link
Collaborator

1 more point: "Environment variable is a potential application wide but not a system wide solution. The user may observe different ringbuffer behaviors among the applications and dlt-daemon."

I would say, libdlt ring buffer will be used by all users, and there already locking mechanism for the data safe, so I do not think that setting an environment variable will lead to different behavior. For instance,by export DLT_USER_RINGBUFFER_MAX=val
We increase the libdlt buffer to that value, and all users will use this buffer until the buffer full. So I think we can make the log rotating option using same way.

add ringbuffer full strategy to control whether remove the oldest log entry or discard new message

on dlt-daemon side in dlt.conf
when RingbufferFullStrategy = 0:    discard the new message
when RingbufferFullStrategy = 1:    remove the oldest entry in the ringbuffer

on application side
export DLT_USER_BUFFER_FULL_STRATEGY=0 to discard new message
export DLT_USER_BUFFER_FULL_STRATEGY=1 to remove the oldest entry in the ringbuffer
@jianjun-huang jianjun-huang force-pushed the delete_oldest_entry_in_ringbuffer branch from 5fa890a to d98c70e Compare January 10, 2024 15:08
@minminlittleshrimp
Copy link
Collaborator

Sorry @jianjun-huang
We are quite busy and late response on this PR.
I am preparing my env at my local and testing this patch.
Will report my result later.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants