Memory leak through missing Closeable

[dr0i]

FileMap is not closeable for it doesn't implement Closeable. Thus the resource is not closed when executing closeStream (either not in Metamorph nor in Metafix.)
As FileMap extends AbstractReadOnlyMap the latter should implement Closeable , guaranteeing a treatment of calling close(). Same is true for RdfMap.

This fixes Memory Leaks we discovered.

The leak appears when a Fix is instantiated several times anew within the same JVM. New instantiations happen e.g. in https://github.com/hbz/rpb/ for every single ETLed document (some handfuls per day). It happens in https://github.com/hbz/lobid-resources/ every time its webhook listener is called (regularly once per day). It happens also in https://metafacture.org/playground/ when clicking dozen of time on process having a Fix loading a table.

Bug fix should be also made for the last java 1.8 version (metafacture-core 5.7.0 and metafacture-fix 0.7.0) since we rely partly on that version , e.g. in rpb.

We should also blog about the Memory Leak: experiencing, discovering, analyzing, fixing.

[dr0i]

@blackwinter @fsteeg how to cope with the idea to have a bug fix in metafacture-fix repo for the java 1.8 version (0.7.1) ? This repo is read only . Should we temporarily unarchive it (if that's possible)?

[dr0i]

Memory leaks appear e.g. in hbz/rpb. Reproduce e.g. via https://github.com/hbz/rpb/blob/fb65fbfa2e2c57f4053a638022b93c02bf575c95/test/rpb/EtlTest.java#L33: put ETL.main in a loop like:

       for (int i=0; i<100; i++) {
            ETL.main(new String[]{"conf/rpb-test-titel-to-strapi.flux"});
            ETL.main(new String[]{"conf/rpb-test-titel-to-lobid.flux"});
            System.out.println(i);
        }

and see how memory consumption increases constantly despite GC operations (these are the green spikes):, resulting in an OutOfMemory error ultimately:

while using #667 memory consumption is flat:

Note also the increased performance over time:
with memory leak: CPU takes 4m22 s when called 60 times ETL.main() vs. only 3m when called 100 times without the memory leak. That is because the GC has to check an ever increasing Memory Heap as the memory leaks. Note also that this performance impact is exponential severe, i.e. the first few instantiations of Fix don't have any noticeable performance impact while at the end, consuming a lot of RAM, the performance is increasingly noticeable (so, it's not likely we will discover performace gains in lobid-resources when doing the basedump because that is done with a freshly startetd JVM. It should consecutively called updates make faster resp. not exponential take more time (as we discover atm)). It may also be that this fixes performance issues we sometimes experience in playground.

[blackwinter]

how to cope with the idea to have a bug fix in metafacture-fix repo for the java 1.8 version (0.7.1) ?

Could you apply the fix locally? Otherwise, we could release from an unpublished branch. Or, as you mentioned, unarchive the repository temporarily.

AIUI, you're blocked from upgrading to a newer (combined) release due to hbz/rpb#129, right?

[dr0i]

Could you apply the fix locally?

I don't like that because it is error prone, especially since we move apps to other server atm. Or WDYT @fsteeg ?

Otherwise, we could release from an unpublished branch.

AFAIK the github releases are also read only (this is the place where we normally release Metafix).

Or, as you mentioned, unarchive the repository temporarily.

If nothing speaks against this, this would be my favourite.

AIUI, you're blocked from upgrading to a newer (combined) release due to hbz/rpb#129, right?

yep

[blackwinter]

I don't like that because it is error prone, especially since we move apps to other server atm.

Totally. I just don't know how long this situation would have to be maintained.

AFAIK the github releases are also read only (this is the place where we normally release Metafix).

Sorry, GitHub Packages is not an option. As I wrote in #577, Maven packages are always scoped to a single repository. This is core now.

We could publish from a metafacture-fix branch in core, though. Not pretty, but should work.

[fsteeg]

Or, as you mentioned, unarchive the repository temporarily.

If nothing speaks against this, this would be my favourite.

Yes, this is probably the best approach, we would get a regular release where it's expected and where we can always find it in the future.

[blackwinter]

Yes, this is probably the best approach, we would get a regular release where it's expected and where we can always find it in the future.

As I said, this is not possible due to a limitation in GitHub Packages.

[fsteeg]

As I said, this is not possible due to a limitation in GitHub Packages.

We can't consume GitHub packages in rpb anyway. I was thinking of a regular tag + release. But even just a tag would be enough, we build a specific version of metafacture-fix anyway. But a release (without publishing packages) would maybe let us find it more easily in the future.

[blackwinter]

We can't consume GitHub packages in rpb anyway.

I see. I assumed that's what you needed because @dr0i was referring to GitHub Packages.

But a release (without publishing packages) would maybe let us find it more easily in the future.

Not sure how important this is, but okay. Let's unarchive metafacture-fix temporarily and backport the fix once it's merged.

Just for the record, though: If you're building from source anyway, you can also apply a patch or build from a fork.

[blackwinter]

backport the fix once it's merged

Wait, this would just be a dependency update in metafacture-fix, right? So we'd have to backport the fix in core and release that first.

BTW: Do we have an official support/backport policy for older releases? Has this ever been done before? Or only now that we have this exceptional situation? ;)

[dr0i]

Do we have an official support/backport policy for older releases?

We haven't. It just came up as need has arisen.

Wait, this would just be a dependency update in metafacture-fix, right? So we'd have to backport the fix in core and release that first.

yes

[fsteeg]

Wait, this would just be a dependency update in metafacture-fix, right? So we'd have to backport the fix in core and release that first.

yes

But like in metafacture-fix, just a tag would work. We can build the core tag, then the fix tag, and depend on the fix version.

[blackwinter]

But like in metafacture-fix, just a tag would work. We can build the core tag, then the fix tag, and depend on the fix version.

But then metafacture-fix would still use the unfixed core version, wouldn't it? Can metafacture-fix consume a tag as a dependency in your setup?

[dr0i]

But then metafacture-fix would still use the unfixed core version, wouldn't it? Can metafacture-fix consume a tag as a dependency in your setup?

Only if we build a release of Metafix. (building MF locally, consume that local build in Metafix and make a release).

However:
Don't you like a regular bug-fix release (mf core 5.7.1) on maven central and a Metafix 0.7.1 on github releases?

[blackwinter]

Only if we build a release of Metafix. (building MF locally, consume that local build in Metafix and make a release).

You can't release a version of metafacture-fix that depends on an unreleased version of metafacture-core. Nobody could use it because the dependency wouldn't resolve.

Don't you like a regular bug-fix release (mf core 5.7.1) on maven central and a Metafix 0.7.1 on github releases?

Are you asking me? I don't really care, TBH. It's an awkward and possibly once-in-a-lifetime situation right now. And as I've said multiple times: We can't release a version of metafacture-fix on GitHub Packages from any repository other than metafacture-core; at least not with the same group ID.

[fsteeg]

Don't you like a regular bug-fix release (mf core 5.7.1) on maven central and a Metafix 0.7.1 on github releases?

Nothing against it, but I also think it's more work than needed. Might still be the cleanest approach though.

And as I've said multiple times: We can't release a version of metafacture-fix on GitHub Packages

Right, but we're talking about GitHub releases, not packages (yeah, it's confusing).

But then metafacture-fix would still use the unfixed core version, wouldn't it? Can metafacture-fix consume a tag as a dependency in your setup?

So my line of thought is: fix the issue in core, tag as 5.7.1, set core dependency in metafix to 5.7.1, tag that as 0.7.1. In the rpb build, we checkout and build both core 5.7.1 and metafix 0.7.1 (which depends on core 5.7.1).

[dr0i]

You can't release a version of metafacture-fix that depends on an unreleased version of metafacture-core. Nobody could use it because the dependency wouldn't resolve.

right - I mixed a a distribution and depencies here, sorry

So my line of thought is: fix the issue in core, tag as 5.7.1, set core dependency in metafix to 5.7.1, tag that as 0.7.1. In the rpb build, we checkout and build both core 5.7.1 and metafix 0.7.1 (which depends on core 5.7.1).

Yep. That's what I meant with "a regular bug-fix release (mf core 5.7.1) on maven central and a Metafix 0.7.1 on github releases".

[fsteeg]

Yep. That's what I meant with "a regular bug-fix release (mf core 5.7.1) on maven central and a Metafix 0.7.1 on github releases".

Well, yes, like I said, that would be fine with me too, but not what I wrote here, which would be just the tags, and building both projects locally (which are then consumed by rpb from the local maven repo).

[blackwinter]

Right, but we're talking about GitHub releases, not packages (yeah, it's confusing).

Okay, I see. I was indeed confused by the fact that this isn't a release that can be consumed as a dependency, which I thought you needed. And GitHub Releases are typically done for both fix and core. But I understand now.

So my line of thought is: fix the issue in core, tag as 5.7.1, set core dependency in metafix to 5.7.1, tag that as 0.7.1. In the rpb build, we checkout and build both core 5.7.1 and metafix 0.7.1 (which depends on core 5.7.1).

So in order to consume metafacture-fix 0.7.1 one has to build metafacture-core 5.7.1 first? That's an unusual workflow as releases go. It's effectively a source-only "release".

In conclusion, do whatever suits your needs. There's probably not going to be anyone else who will reach for this release.

[dr0i]

@fsteeg oh, you are right. These are different things (should go home, too tired to read/understand properly).