Memory leak SAK-21398

Activity

SAKTRUNK-17 15

Keyboard shortcuts  
Summarize the review outcomes (optional)
 
#permalink

Details

Warning: no files are visible, they have all been filtered.
Participant Role Time Spent Comments Latest Comment
Author 51m 7 Thanks for the info, Earle. Maybe next time! The method I...
Reviewer - 100% reviewed 8m 1 Looked good
Aaron Zeckoski (deleted user)
Reviewer - 0% reviewed      
Reviewer - 100% reviewed 3m 1 This should be M_log
Reviewer - Complete 9m 1 In the future if you have a task like this you should try...
Reviewer - 100% reviewed 8m 1 looks good to me
Reviewer - 100% reviewed 8m 1 I'd rename and prefix this with chat, something like chat...
Reviewer - 100% reviewed 5m 1 I think your approach is pretty straightforward.
Greg Thomas (deleted user)
Reviewer - 100% reviewed 5m    
Reviewer - Complete 8m 2 What is the effect on users when deliveries are removed? ...
Total   1h 45m 15  
#permalink

Objectives

Addressing the memory leak mentioned in https://jira.sakaiproject.org/browse/SAK-21398

Branches in review

#permalink

Issues Raised From Comments

Key Summary State Assignee
#permalink

General Comments

Chris Maurer

I always have problems with a diff of a new file, so Expirable.java is just a...

I always have problems with a diff of a new file, so Expirable.java is just attached, but it would be in courier/api

Chris Maurer

And anyone is welcome to review this. I just picked some folks that it sugges...

And anyone is welcome to review this. I just picked some folks that it suggested!

Chris Maurer

After some initial testing we're thinking about running the maint thread ever...

After some initial testing we're thinking about running the maint thread every 5 minutes and having the ttl on the delivery also be 5 minutes.

Bryan Holladay

Looked good

Looked good

Chris Maurer

And I'll add the props docs to the default.sakai.properties before committing...

And I'll add the props docs to the default.sakai.properties before committing too.

Seth Theriault

I think your approach is pretty straightforward.

I think your approach is pretty straightforward.

Earle R Nietzel

In the future if you have a task like this you should try using a TimerTask o...

In the future if you have a task like this you should try using a TimerTask or even the more modern ScheduledExecutorService, they are geared for these types of tasks.

See reading material!
http://www.javapractices.com/topic/TopicAction.do?Id=54

http://stackoverflow.com/questions/409932/java-timer-vs-executorservice

Chris Maurer

Thanks for the info, Earle. Maybe next time! The method I used was already in...

Thanks for the info, Earle. Maybe next time! The method I used was already in place elsewhere in Sakai (not that that means it's always acceptable behavior) so I just went that route.

John Bush

looks good to me

looks good to me

David Haines

What is the effect on users when deliveries are removed? Do they just not see...

What is the effect on users when deliveries are removed? Do they just not see some of the chat updates?

/chat/chat-tool/.../tool/ChatDelivery.java Changed   1
Open in IDE #permalink
/courier/.../impl/BasicCourierService.java Changed   4
Open in IDE #permalink
/courier/courier-impl/impl/pom.xml Changed  
Open in IDE #permalink
/Expirable.java Added
Open in IDE #permalink

Review updated: Reload | Ignore | Collapse

You cannot reload the review while writing a comment.

Create issue

X
Assign To Me

Log time against