Good morning, I addressed the minor comments from ...
# request-review
j
Good morning, I addressed the minor comments from @Jim Downie , if anyone would mind giving the final ✅ on his behalf, of this version topic/version bump PR: https://github.com/nf-core/modules/pull/11379
👀 1
m
j
Any suggestion how to fix mine? https://github.com/nf-core/createtaxdb/pull/191
👀 1
Already tried resolving on github codespaces 😞
OH WAIt
m
No worries, I can help out
j
No no I think you have
one moment
I saw maybe the difference in your PR 😉
Hrmm or not...
m
I am the self nominated template merge master champion
😆 2
j
AHA I FIXED IT
m
java.util.ConcurrentModificationException
?
j
Yeah, random error that only happens half the time on the test data...
Not sure why
m
interesting
For me, TEMPLATE is the guidelines to follow except when you think you have a better solution
👍 1
If there's a rule, there's a way around
😆 1
If you see that error often, you could add a blank retry strategy for this process
j
I could do, but I want to actually sit down and investigate WHY it happens
✅ 1
m
smarter
j
Essentially, the ConcurrentModificationException is used to fail-fast when something we are iterating on is modified.
https://www.baeldung.com/java-concurrentmodificationexception Something is being modified somewhere when it shouldn't be - would take a look at anywhere you have iterations via
collect
or meta-map modification, maybe?
👍 1
j
YEah I had such an idea but never worked out what it was 😞
I don't know if it's from a file, or a metamap, or something within the process
And it's variable WHERE it's happening
j
The fact that it happens only sometimes suggests it's a race condition?
100 rainbow 2
j
Most often in the
FIND
, but occasioanlly in downstream processes
j
Have you tried pointing an LLM at it? They can be quite shoddy with Nextflow but might be able to point you to the right kind of location
j
I tried SeqeraAI end of last year and didn't help
Could try again...
unless you have a suggestion of a bette rLLM for such a thing? My experience of them has been mostly crap
stioll
j
No, generally they really suck hard at Nextflow
this 1
Very good for general Groovy though
👍 1
It's probably completely off-base but I pointed Copilot at the problem and it doesn't like the flatten here in the preprocessing subWF:
Copy code
ch_prepped_aa_fastas_renamed = SEQKIT_BATCHRENAME.out.fastx
                .map { _meta, fasta -> [[id: params.dbname], fasta] }
                .groupTuple()
                .map { meta, fasta ->
                    [meta, fasta.flatten()]
                }
j
I don't think so, SEQKIT is only applied for Kaiju and there should only be a single instance of it I think 🤔
And shouldn't the
fasta
element be independent at this point 🤔
SeqeraAI is still going at least
Took an hour, but it apparently did this:
Copy code
Summary

The java.util.ConcurrentModificationException errors in the nf-core/createtaxdb pipeline are caused by concurrent access to shared collections and file staging conflicts. Here's what I found and fixed:
Root Causes:

    Complex channel operations with groupTuple(), map(), and join() creating race conditions when multiple processes access the same data structures
    Shared staging directories where multiple processes try to stage files to the same location simultaneously
    Lack of process-level synchronization for critical operations

Fixes Applied:

    Process Synchronization: Added maxForks = 1 for FIND processes to serialize them, and maxForks = 2 for MALT_BUILD/METACACHE_BUILD to limit concurrency

    Unique Staging Directories: Modified staging paths to use ${task.index} to ensure each task gets its own staging directory

    Retry Logic: Added automatic retry mechanisms with errorStrategy and maxRetries = 3 for all problematic processes

    Error Handling: Improved error handling to gracefully handle and retry failed tasks

Expected Results:

    Elimination of ConcurrentModificationException errors
    Improved reliability through automatic retries
    Maintained functionality with minimal performance impact
    Better resource management through controlled concurrency

The fixes maintain the original pipeline functionality while addressing the concurrency issues that were causing the sporadic failures. The FIND processes will now run sequentially (preventing race conditions), and the build processes use unique staging directories to avoid file conflicts.

I'm done.
I don't know WHERE it's applied these fixes though 😅
OK setting
maxForks = 1
for everything does seem to fix this though 😅
Hrmmm it did just inspire an idea, it seems to be the same source channel that always triggers it... 🤔
The failures always comes from the
ch_prepped_dna_fastas
bit here:
Copy code
// Unbatch the unzipped files for rematching with metadata
        ch_prepped_dna_fastas_gunzipped = ch_prepped_dna_batches
            .flatMap { _meta, batch -> batch }
            .map { fasta -> [fasta.getName(), fasta] }

        // Match metadata back to the prepped DNA fastas with an inner join
        ch_prepped_dna_fastas_ungrouped = ch_prepped_dna_fastas_gunzipped
            .join(ch_dna_refs_for_rematching, failOnMismatch: true, failOnDuplicate: true)
            .map { _fasta_name, fasta, meta -> [meta, fasta] }

        // Prepare for making the mega file
        ch_prepped_dna_fastas = ch_prepped_dna_fastas_ungrouped
            .map { _meta, fasta ->
                [[id: params.dbname], fasta]
            }
            .groupTuple()
Was there a way to have a operation to 'wait' until it had finished to continue? 🤔
m
to wait?
what is your issue there?
j
No wait that's not what I meant sorry
The
ConcurrentModificationException
, with processes trying to pull the same thing
at the same time
m
Oh I see
j
I ahve like 5-6 processes all trying to call the same channel at once
m
is it a file that needs to be staged?
j
Yes
I guess
The raceconditions occur just as a process starts (so no exit condition)
m
In such a case, I would try to make sure only one process has access to it first
j
And the processes actually share the same input channel (in some cases iwth a slightly different name as they are passed out of a subworkflow)
Yes, but how 😬
I wonder how to stagger that
Seqera AI suggsted maxForks (but this is not really feasible
m
Make a copy of the input channel, and try to join it with the output of 1 of these process, and use it as input for all your other processes
j
Or
Copy code
Unique Staging Directories: Modified staging paths to use ${task.index} to ensure each task gets its own staging directory
But that sounds dodgy to me
j
Is it explicitly always the FIND_CONCATENATE processes that fail?
j
Make a copy of the input channel, and try to join it with the output of 1 of these process, and use it as input for all your other processes
Wouldn't that essentially make the whole thing linear?
Is it explicitly always the FIND_CONCATENATE processes that fail?
Most commonly, but that's what I just realised: METACACHE and MALT and the others that it occasioanlly comes up all share the same input channel
(sometimes with a different name, but same contents)
j
I would be very surprised if it was just processes consuming the same channel that causes the issuue
j
Same
But that is the common thing here
And it's happening at the point where the process is trying to stage things
j
Maybe you need to go back to @Ben Sherman and the Github issue? If it's occurring during staging that sounds like a bug?
party this 1
Looking at your workflow code nothing immediately stands out to me
j
Yeah me neither
j
Other thing to try is the "comment it all out" approach
j
Yeah.. I tried going a bottom up approach with a minimal reprex but didn't help
j
I would literally comment out the whole pipeline, then introduce 1 process that reads the channel, then a second, etc. - until you can replicate the error
😢 1
m
MRE