> we decided to limit the number of testing forks/disable fork reuse
for almost all tests
IT cases were generally not reusing forks for as long as I can remember.
A few modules opted into fork-reuse; most didn't.
The Table API recently enabled fork-reuse, started causing OOMs due to
memory leaks, which have since been fixed and fork-reuse has been
enabled again.
> machine that has 7GB of RAM in total
This is indeed the case for the azure-provided machines (used for e2e
tests and contributor builds).
The machines we use for apache/flink / PRs have 64GB ram in total,
spread over 7 agents, so about ~8 per agent and roughly equivalent to
the azure machines if you account for some overhead.
I'm not aware of any explicit assignment of off-heap memory.
This has worked fine in the past, and anytime we ran into OOM errors we
were eventually able to pinpoint the cause to some memory leak in Flink.
I thus assume that the OOMs we've recently seen are again due to some
new leak.
Overall I appreciate the sentiment of using less resources for unit
tests, but I don't see how it would really address the issue.
You still have the potential for all agents running ITCases at the same
time, leaving us in the status-quo.
It could reduce the likelihood of OOMs, but so long as leaks are there
it is inevitable that we hit one.
(And as mush as I hate it, if there is a leak I'd kinda prefer that to
be more visible, as it seems the only way to get people to look into
leaks at all is getting into a situation where everyone is seriously
annoyed.)
Instead we could try decreasing the general memory usage slightly (even
256mb less would give us a nice buffer on the alibaba machines),
and I would hazard a guess that it'd barely reduce testing times (since
most ITCases shouldn't hit that limit anyway).
I do agree that in the long-term we should strive towards re-using
forks, and I know that Francesco also looked into executing test cases
in parallel using JUnit5 to speed things up.
But there are a number of classes that make this quite difficult;
singletons like FileSystem/GlobalConfiguration, anything using static
stuff like context environments (not sure if this still applies), legacy
testing code, class-loading bugs in libraries etc etc.
I would actually split ITCases into 2 categories depending on whether
they work with fork-reuse.
On 31/03/2022 14:48, Piotr Nowojski wrote:
Hi devs,
Recently we were plagued with OOM errors and as a result we decided to
limit the number of testing forks/disable fork reuse for almost all tests.
Currently AFAIK each JVM fork has assigned 4GB of memory (2GB heap and 2GB
off heap) and we are running two forks, on a machine that has 7GB of RAM in
total.
What I would like to discuss is to introduce test categories, for example
small and large tests, where small tests would require less memory and thus
we would be able to increase the number of forks for them. We could
probably achieve this differentiation via different means (annotations,
test naming conventions, etc), but I would propose to just re-use our
pre-existing convention of suffixing more "unity" testing classes with
`Test` suffix, while more "integration like" classes with `ITCase` suffix.
We could keep running `ITCase`'s with two forks 4GB memory each, while for
example unit tests we could run with four forks 2GB memory each.
What do you think?
Best,
Piotrek
