New and Improved 3.13 Beta. Highlights: Shareable filters and dashboards and lots of other goodies. Any feedback can be raised as JIRA issues in the JIRA project.
Issue Details (XML | Word | Printable)

Key: JRA-7656
Type: Improvement Improvement
Status: Resolved Resolved
Resolution: Fixed
Priority: Major Major
Assignee: Dylan Etkin [Atlassian]
Reporter: Nick Minutello
Votes: 0
Watchers: 0
Operations

If you were logged in you would be able to see more operations.
JIRA

CVS Log write-lock-file implementation leads to trouble....

Created: 16/Aug/05 08:42 AM   Updated: 04/Jul/06 08:47 PM
Component/s: CVS integration
Affects Version/s: 3.2.3
Fix Version/s: 3.4

Time Tracking:
Not Specified

File Attachments: 1. Java Source File CvsRepositoryUtilImpl.java (18 kB)
Issue Links:
Duplicate
 

Participants: Anton Mazkovoi [Atlassian], Dylan Etkin [Atlassian], Nick Menere [Atlassian] and Nick Minutello
Since last comment: 2 years, 37 weeks ago
Resolution Date: 24/Oct/05 09:16 PM
Labels:


 Description  « Hide
Basically, when you set the path to the log file, Jira wont create any directories that are required - so you tend to put all the cvs log files in the same directory.

However, the write-lock-file implementation :

final Lock lock = getLock(logFile.getParentFile().getAbsolutePath());

means that the lock-file then becomes global - for all cvs-logs - .... because all log files are in the same directory

You want to:
1) create any directories that are required when saving the log file
2) make the lock-file name contain the log-file name - so that each cvs-log file has its own lock file.... even if they are all in the same directory

 All   Comments   Work Log   Change History      Sort Order: Ascending order - Click to sort in descending order
[ Permlink | « Hide ]
Nick Minutello added a comment - 16/Aug/05 09:06 PM
Chaps, I am going to have to patch this - I will send you the code...

[ Permlink | « Hide ]
Anton Mazkovoi [Atlassian] added a comment - 16/Aug/05 10:06 PM
Fair enough.

Just ofr the record, may I ask when this caused you trouble? Is it while updating module details in the admin section?

Anton


[ Permlink | « Hide ]
Nick Minutello added a comment - 16/Aug/05 10:15 PM
Not sure what happenned - but the lock file has been left lying around - so none of our modules have been updating since Aug-12

[ Permlink | « Hide ]
Nick Minutello added a comment - 17/Aug/05 07:19 PM
Also seems as though the auto-updating has also stopped...
We need to restart jira - but we cant yet... (hard to find a quiet time)

[ Permlink | « Hide ]
Anton Mazkovoi [Atlassian] added a comment - 17/Aug/05 07:26 PM
Do you mean that after you removed the lock file the Vcs Service is not updating the repos?

Can you try setting the log level to DEBUG:
http://www.atlassian.com/software/jira/docs/v3.2.3/log_and_prof.html
and see if the service is running?

If it is, and its 'stuck' can you take a thread dump and see what its up to?

Thanks,
Anton


[ Permlink | « Hide ]
Nick Minutello added a comment - 17/Aug/05 07:30 PM
Yep, removing the lock file still left it stuck.
I didnt have time to look into it (was with Mike) - but I will get a thread-dump for you
-N

[ Permlink | « Hide ]
Nick Minutello added a comment - 24/Aug/05 08:12 PM
It got stuck again.

Seems the offending thread stack is:

3XMTHREADINFO "JiraQuartzScheduler_Worker-2" (TID:4066B0C0, sys_thread_t:9A0BFD0, state:R, native ID:9820) prio=4
4XESTACKTRACE at java.lang.Thread.sleep(Native Method)
4XESTACKTRACE at org.netbeans.lib.cvsclient.util.LoggedDataInputStream.readByte(Unknown Source)
4XESTACKTRACE at org.netbeans.lib.cvsclient.Client.handleResponse(Unknown Source)
4XESTACKTRACE at org.netbeans.lib.cvsclient.Client.processRequests(Unknown Source)
4XESTACKTRACE at org.netbeans.lib.cvsclient.command.log.RlogCommand.postExpansionExecute(Unknown Source)
4XESTACKTRACE at org.netbeans.lib.cvsclient.command.log.RlogCommand.execute(Unknown Source)
4XESTACKTRACE at org.netbeans.lib.cvsclient.Client.executeCommand(Unknown Source)
4XESTACKTRACE at com.atlassian.jira.vcs.cvsimpl.CvsRepositoryUtilImpl.updateCvs(CvsRepositoryUtilImpl.java:312)
4XESTACKTRACE at com.atlassian.jira.vcs.cvsimpl.CvsRepository.updateCvs(CvsRepository.java:204)
4XESTACKTRACE at com.atlassian.jira.vcs.cvsimpl.CvsRepository.updateRepository(CvsRepository.java:295)
4XESTACKTRACE at com.atlassian.jira.vcs.DefaultRepositoryManager.updateRepository(DefaultRepositoryManager.java:492)
4XESTACKTRACE at com.atlassian.jira.vcs.DefaultRepositoryManager.updateRepositories(DefaultRepositoryManager.java:445)
4XESTACKTRACE at com.atlassian.jira.service.services.vcs.VcsService.run(VcsService.java:54)
4XESTACKTRACE at com.atlassian.jira.service.JiraServiceContainer.run(JiraServiceContainer.java:59)
4XESTACKTRACE at com.atlassian.jira.service.ServiceRunner.execute(ServiceRunner.java(Compiled Code))
4XESTACKTRACE at org.quartz.core.JobRunShell.run(JobRunShell.java:191)
4XESTACKTRACE at org.quartz.simpl.SimpleThreadPool$WorkerThread.run(SimpleThreadPool.java(Compiled Code))


[ Permlink | « Hide ]
Nick Minutello added a comment - 24/Aug/05 09:05 PM
Its not going to necessarily fix the stuck thread.....

But I have made the following small change(s) to CvsRepositoryUtilImpl:

-------------
private static final String LOCK_FILE_NAME_SUFFIX = ".write.lock";
-------------
private Lock getLock(File logfile)

{ return new Lock(logfile.getParent(), logfile.getName() + LOCK_FILE_NAME_SUFFIX); }

-------------
public CvsContent parseCvsLogs(...)
final Lock lock = getLock(logFile);
}
-------------
public void updateCvs(...)
// Create a lock to 'show' that we are updating the CVS log file
Lock lock = getLock(logFile);
}


[ Permlink | « Hide ]
Nick Minutello added a comment - 24/Aug/05 09:08 PM
To fix the stuck thread, you may want to use the concurrency utils - and use a FutureTask - which you can wait for completion for a timeout period - and if it hasnt completed in, say, 10 mins, you can cancel it.
And with a threadpool executor, this would also let you run a couple in parallel (but not too many for the poor cvs server...)
-N

[ Permlink | « Hide ]
Anton Mazkovoi [Atlassian] added a comment - 25/Aug/05 12:52 AM
We'll try to commit the patch for 3.4 - not sure if we will be able to get the concurrency stuff in as well though.

[ Permlink | « Hide ]
Nick Minutello added a comment - 25/Aug/05 04:03 AM
I will add the concurrency stuff to our production server - and once its tested, post the code here (the code for that is pretty straightforward...)

[ Permlink | « Hide ]
Anton Mazkovoi [Atlassian] added a comment - 25/Aug/05 04:06 AM
Sounds good to me Thanks!

[ Permlink | « Hide ]
Nick Minutello added a comment - 10/Sep/05 10:02 AM
Voila!
CVS operation with timeout.
I couldnt write any unit tests without refactoring it - but I have tested it end-to-end.

[ Permlink | « Hide ]
Anton Mazkovoi [Atlassian] added a comment - 11/Sep/05 07:06 PM
Thanks! We'll see if we can get this into 3.4

[ Permlink | « Hide ]
Nick Minutello added a comment - 20/Oct/05 03:51 AM
Guys, with the implementation as it stands now, we have to update it manually.
Within a few days, the CVS thread hangs - the lock file sits there and we cant even update it manually.
Its a bit dissappointing to have written the code for you - and it has to wait until 3.5 to go in??

[ Permlink | « Hide ]
Anton Mazkovoi [Atlassian] added a comment - 20/Oct/05 03:56 AM
We are going to revisist a few things in regards to CVS in 3.5, so it would be a logical place to do this. If this is very important to you we will merge into 3.4.

[ Permlink | « Hide ]
Dylan Etkin [Atlassian] added a comment - 24/Oct/05 09:16 PM
I have committed and tested this patch. Thanks for the work Nick.

[ Permlink | « Hide ]
Nick Minutello added a comment - 24/Oct/05 10:23 PM
Cheers. Thanks. Its going to make 1.4?
-Nick

[ Permlink | « Hide ]
Nick Menere [Atlassian] added a comment - 24/Oct/05 10:33 PM
Nick,

It will be in 3.4.

Cheers,
Nick


[ Permlink | « Hide ]
Nick Minutello added a comment - 25/Oct/05 05:18 AM
cool

[ Permlink | « Hide ]
Nick Minutello added a comment - 13/Dec/05 08:30 PM
Ouch. Not sure if you noticed, but my last test was complete shite.

Should have been

String processedComment = regexCommentHandler.splitMailBody(rawEmailbody);

(rather than regexCommentHandler.splitMailBody(expectedComment))

Not entirely surprising, when you make that change, the test no longer passes.
You need to change the regex to the following

params.put(KEY_REGEX, "/Extranet\\s*jira.london.echonet/FMB/UK/EUROPE/GROUP@CRAPPYLOTUSNOTES/");

[ Permlink | « Hide ]
Anton Mazkovoi [Atlassian] added a comment - 13/Dec/05 08:56 PM
Nick,

Is the last comment meant to be on JRA-8319?

Thanks,
Anton


[ Permlink | « Hide ]
Nick Minutello added a comment - 14/Dec/05 04:36 AM
Er, yes. Woops.


Powered by Atlassian JIRA the Professional Issue Tracker. (Enterprise Edition, Version: 3.13_beta1-#328) - Bug/feature request - Atlassian news - Contact Administrators