Skip to content

Feature/bergel1/lumberjack stream ljvec#1805

Draft
gberg617 wants to merge 2 commits intodevelopfrom
feature/bergel1/lumberjack-stream-ljvec
Draft

Feature/bergel1/lumberjack stream ljvec#1805
gberg617 wants to merge 2 commits intodevelopfrom
feature/bergel1/lumberjack-stream-ljvec

Conversation

@gberg617
Copy link
Contributor

Summary

The goal of this PR is to:

  • Use multiple Lumberjack objects within the same LumberjackStream instance, each with its own combiner criteria.
  • Log messages from those Lumberjack objects to the same file.
  • Preserve a global sort by timestamp across all messages for a given file managed by a LumberjackStream instance.

The key ideas are:

  • LumberjackStream now holds a vector of Lumberjack objects instead of a single one.
  • Each Lumberjack object can be configured to only process messages with certain tags, if tags are provided.
  • The sorting responsibility moves from Lumberjack to LumberjackStream, which gathers messages from all of its Lumberjack instances and performs a single sort by timestamp before writing.

This avoids the expensive duplicate checking for tagged messages that are not intended to be combined, and still allows:

  • Sorting all messages that go to a given file.
  • Using different combiner strategies for different tags.

@gberg617 gberg617 self-assigned this Feb 18, 2026
@gberg617 gberg617 marked this pull request as draft February 18, 2026 22:17

//------------------------------------------------------------------------------
bool LumberjackStream::hasPendingMessages() { return m_lj->getMessages().size() > 0; }
bool LumberjackStream::hasPendingMessages() {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This needs to be fixed.

bool LumberjackStream::isUsingMPI() { return true; }

MPI_Comm LumberjackStream::comm() { return m_lj->getCommunicator()->comm(); }
MPI_Comm LumberjackStream::comm() { return m_lj[0]->getCommunicator()->comm(); }
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This also needs to be fixed.

@bmhan12
Copy link
Contributor

bmhan12 commented Feb 18, 2026

Thanks for clearly laying out the issue, @gberg617 !

I wonder if instead of having multiple LumberjackStream objects each corresponding to different tags that filter out different messages to queue by queueMessage(), we can stick with a single LumberjackStream object and have a blacklist vector of tag(s) that do not get combined in combineMessages()? (link to combineMessages() function)

So instead of these non-combinable messages being added to finalMessages and iterated over repeatedly, they get added to a separate vector i.e.uncombinableMessages and get appended back to m_messages at the very end?

@gberg617
Copy link
Contributor Author

Thanks for clearly laying out the issue, @gberg617 !

I wonder if instead of having multiple LumberjackStream objects each corresponding to different tags that filter out different messages to queue by queueMessage(), we can stick with a single LumberjackStream object and have a blacklist vector of tag(s) that do not get combined in combineMessages()? (link to combineMessages() function)

So instead of these non-combinable messages being added to finalMessages and iterated over repeatedly, they get added to a separate vector i.e.uncombinableMessages and get appended back to m_messages at the very end?

@bmhan12 That's a good point. I think your idea can work. The advantage of your approach is that we don't change the behavior in LumberjackStream, and all the changes stay localized within Lumberjack. However, this would require me to keep the custom Combiner class I wrote in my host code that inspects the tag to determine whether messages should be combined. At first, this seemed somewhat undesirable, but perhaps it's still better than modifying LumberjackStream.

@gberg617
Copy link
Contributor Author

Thanks for clearly laying out the issue, @gberg617 !
I wonder if instead of having multiple LumberjackStream objects each corresponding to different tags that filter out different messages to queue by queueMessage(), we can stick with a single LumberjackStream object and have a blacklist vector of tag(s) that do not get combined in combineMessages()? (link to combineMessages() function)
So instead of these non-combinable messages being added to finalMessages and iterated over repeatedly, they get added to a separate vector i.e.uncombinableMessages and get appended back to m_messages at the very end?

@bmhan12 That's a good point. I think your idea can work. The advantage of your approach is that we don't change the behavior in LumberjackStream, and all the changes stay localized within Lumberjack. However, this would require me to keep the custom Combiner class I wrote in my host code that inspects the tag to determine whether messages should be combined. At first, this seemed somewhat undesirable, but perhaps it's still better than modifying LumberjackStream.

@bmhan12 After thinking about this, I slightly favor the approach in the current PR. I think it is generally useful to let a single LumberjackStream instance cleanly handle different combiner strategies for one file, instead of creating multiple LumberjackStream objects that all own the same file but use different combiners, or relying on custom Combiner classes like I have been doing. Moving the message sorting responsibility from Lumberjack into LumberjackStream in slic also improves the separation of responsibilities. LumberjackStream is now responsible for formatting and writing sorted messages to a file, and Lumberjack focuses on combining and passing messages between ranks. if you feel this isn't the best approach, I'm happy to discuss it further.

@bmhan12
Copy link
Contributor

bmhan12 commented Feb 20, 2026

@gberg617 Agreed, separation of responsibilities is a good general practice/design, and having multiple Lumberjacks can helps make things clearer (one Lumberjack for tag A messages, another for tag B messages, etc.).

Two things I am concerned about, open to thoughts/discussion:

Additional MPI overhead as we increase the number of Lumberjacks. Each Lumberjack object adds additional MPI operations to push messages to root, each Lumberjack object having its own set of messages to manage. I don't know concretely what the overhead is, this might not matter at all.

Overhead from processing non-combinable messages separately. With the way combineMessages() is written (link to combineMessages() function), if we filter all non-combinable messages to their own Lumberjack object, the function still does the expensive duplicate checking for that Lumberjack object. The current implementation of combineMessages() assumes most, if not all, messages to Lumberjack are intended to be combined.

We would need to add a blacklist/blocklist like suggested or some other way of indicating the messages for this Lumberjack object in particular do not need to be combined.

But based on the point of separation of responsibilities/ Lumberjack's role being to combine messages, if in this scenario we do not want to combine messages, maybe we should not be using Lumberjack/LumberjackStream on these non-combinable messages? SynchronizedStream makes more sense if we need to pass messages between ranks without combining. I'm assuming you know beforehand if a message is combinable or not so you , that assumption might not be realistic.

FYI - I added a test branch that implements the blocklist: https://github.com/llnl/axom/tree/feature/han12/lj_blocklist. Feel free to try it out!

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.

2 participants