search

greenmail-mail-test / greenmail

Official master for the Greenmail project
http://greenmail-mail-test.github.io/greenmail/
Apache License 2.0
630 stars 181 forks source link

Provide ability to automatically purge oldest messages. #62

Open Chessray opened 9 years ago

Chessray commented 9 years ago

When running an email simulator based on GreenMail for too long (or rather when it has received too many messages), it will invariably die with an OutOfMemoryError. Is it possible to change e.g. the ImapHostManagerImpl to only store a maximum number of messages and when it's supposed to receive more, it will purge its oldest entry?

camann9 commented 9 years ago

I can feel your pain since I had a similar issue with Wiremock. Would you mind providing a pull request for this.

Chessray commented 9 years ago

Not sure if I can get to this anytime soon, as I'm in the process of moving house. Lots of stuff to pack, then unpack for weeks. If nobody picks this up, I'll get to it afterwards.

Chessray commented 9 years ago

OK, looks like I'll start this out myself now. Just one question: We solved similar issues in our other simulators by using Guava's EvictingQueue. I see that Guava is not yet in the list of dependencies. Would adding this be acceptable? Alternatively, there's a class CircularFifoQueue in Apache Commons Collections.

But I'd like to go ahead with Guava, if there are no objections.

camann9 commented 9 years ago

Hi Raimund, we try to keep the list of dependencies short to prevent dependency hell. After all we are just a testing helper library so we don't want to cause trouble to people. It would be great if you could manage without additional dependencies. Just create your own small class for the queue. I know that library use is good but we are even considering removing our current dependency on Apache Commons (which we only use in one place anyways)... @marcelmay what do you think?

On Fri, May 1, 2015, 4:40 PM Raimund Klein notifications@github.com wrote:

OK, looks like I'll start this out myself now. Just one question: We solved similar issues in our other simulators by using Guava's EvictingQueue http://docs.guava-libraries.googlecode.com/git-history/release/javadoc/com/google/common/collect/EvictingQueue.html. I see that Guava is not yet in the list of dependencies. Would adding this be acceptable? Alternatively, there's a class CircularFifoQueue http://commons.apache.org/proper/commons-collections/javadocs/api-release/org/apache/commons/collections4/queue/CircularFifoQueue.html in Apache Commons Collections.

But I'd like to go ahead with Guava, if there are no objections.

— Reply to this email directly or view it on GitHub https://github.com/greenmail-mail-test/greenmail/issues/62#issuecomment-98146170 .

buildscientist commented 9 years ago

Couple of notes on this:

buildscientist commented 9 years ago

@Chessray

Have you considered setting a quota per GreenMail user and then flushing the IMAP mailbox once that quota is reached? That's a higher level implementation of what you're trying to do with the EvictingQueue but from an email simulator/test standpoint - it might make a lot more sense.

buildscientist commented 9 years ago

Ok I just looked into this - and wrote a quick POC that does actually delete all messages across a given mailbox. @Chessray if you like I can add a nice helper method in GreenMailUtil where you pass the name of the mailbox you want and it'll delete all the messages.

The code I came up is as follows: 


import com.icegreen.greenmail.Managers;
import com.icegreen.greenmail.imap.ImapHostManager;
import com.icegreen.greenmail.store.InMemoryStore;
import com.icegreen.greenmail.store.MailFolder;
import com.icegreen.greenmail.util.ServerSetupTest;

public static void removeAllMessagesInMailbox(GreenMail greenmail,String mailbox) {
        Managers manage = greenMail.getManagers();
        ImapHostManager imaphost = manage.getImapHostManager();
        InMemoryStore store = (InMemoryStore) imaphost.getStore();
        MailFolder folder = store.getMailbox(mailbox);
        folder.deleteAllMessages();
}
Chessray commented 9 years ago

Hi again,

OK, so no additional dependencies. @buildscientist As for the helper method: This looks nice, but doesn't solve the scenario why I came up with this in the first place:

// Given Email test server based on GreenMail is running all day long in the background, picking up an email here and there. (No big deal)

// When Tester runs a mass email test a few times.

// Then Email test server falls over with out of memory. Tester reports a failure. @Chessray frustratedly restarts mail server and curses the world. Both teammembers have wasted a considerable amount of worktime.

The idea is of course that the tester sees the emails he has sent with his last test, or at least a few of them. Full flushes won't help with that, I'm afraid. We have simulators for other channels (e.g. SMS, Mobile Push) that face similar issues, and we have used the EvictingQueue. The docs say it's not thread-safe, but I believe this only means you can't guarantee an order. We have a UnitTest firing loads of messages at it from several parallel threads, and it's working fine.

camann9 commented 9 years ago

So I think you should implement this, it seems like a very useful feature to me. Just go ahead. The idea of integrating this with quota is interesting but I wouldn't do it since we are taking about two very different functionalities. Just add an option for the maximum number of messages and implement a simple version of EvictingQueue yourself. I'm convinced it's not more than 30 LOC.

On Mon, May 11, 2015, 12:44 AM Raimund Klein notifications@github.com wrote:

Hi again,

OK, so no additional dependencies. @buildscientist https://github.com/buildscientist As for the helper method: This looks nice, but doesn't solve the scenario why I came up with this in the first place:

// Given Email test server based on GreenMail is running all day long in the background, picking up an email here and there. (No big deal)

// When Tester runs a mass email test a few times.

// Then Email test server falls over with out of memory. Tester reports a failure. @Chessray https://github.com/Chessray frustratedly restarts mail server and curses the world. Both teammembers have wasted a considerable amount of worktime.

The idea is of course that the tester sees the emails he has sent with his last test, or at least a few of them. Full flushes won't help with that, I'm afraid. We have simulators for other channels (e.g. SMS, Mobile Push) that face similar issues, and we have used the EvictingQueue. The docs say it's not thread-safe, but I believe this only means you can't guarantee an order. We have a UnitTest firing loads of messages at it from several parallel threads, and it's working fine.

— Reply to this email directly or view it on GitHub https://github.com/greenmail-mail-test/greenmail/issues/62#issuecomment-100799539 .

buildscientist commented 9 years ago

@camann9 Sounds good. I'll create another enhancement and implement the util method to flush mailboxes.

@Chessray - Sorry about your frustration. Are you planning on implementing this?

I'd say it is more than 30 LOC as we're effectively implementing another Store (InMemoryEvictingStore?)

If I had to plan this out I'd say you'd be touching the following classes:

And adding a class that implements Store *InMemoryEvictingStore

And another class that implements the circular/evicting queue *EvictingQueue

I am not a Java concurrency expert but I'd say that with the EvictingQueue not being thread safe would have a larger impact than out of order messages. Any write operations (changing the state of a mail envelope for example) could potentially lead to some unexpected behavior. Perhaps @marcelmay and @camann9 can speak to this.

Chessray commented 9 years ago

Yes, I'll try to take this on now. Let's see if I can cheat myself out of writing a completely new class by resorting to a simple LinkedHashMap extension instead of a full blown Queue implementation.

marcelmay commented 9 years ago

@Chessray ,

I'm interested in the cause of the memory bloat. Maybe it is possible to reduce the memory footprint (as an alternative approach, eg by offloading attachments to tmp files).

We did have an issue #18 with hanging threads, too (so thread stack size could be a cause, too).

Chessray commented 9 years ago

Hi @marcelmay,

I think it's merely the sheer amount of emails. AFAIK we don't send any attachments. We provide an option for sending messages to millions of registered recipients at once (who just said Spam?). At some point, the InMemoryStore is bound to break, just like it happened to our other simulators (Push, SMS). As for the version, we're currently on 1.3.1b.

Basically, we have a test system running with as much uptime as possible. Since we also have to test NFRs which include large mail volumes, our GreenMail-based simulator must be able to handle this. As we are typically only interested in the emails from the latest test, we're happy with older ones being purged automatically - even with some from the latest request purged immediately, as long as there still are enough for us to see the texts that were sent in different languages.

I can't say much about the VM heapspace, but it's certainly nontrivial.

buildscientist commented 8 years ago

What's the status of this issue? Can we close this?

marcelmay commented 8 years ago

Still open, I assigned it to myself.

Chessray commented 8 years ago

@marcelmay The pull request is already there, please just review and merge. Sorry that I didn't pay attention to this anymore lately - I've been assigned to a completely new project, so I felt no more pressure to keep an eye on this. ;)