Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NIFI-6895: Fix PutKudu processor concurrency issues #3910

Closed
wants to merge 1 commit into from

Conversation

granthenke
Copy link
Member

Calls to trigger() may be called concurrently from different threads,
however the PutKudu processor is storing the kuduSession
in a class level field. This can result in the logging issue reported in
NIFI-6895 and likely other unusual anomolies including performace
issues depending on the processor configuration.

Additionally the operationType was also stored in a class level field
and could be set concurrently resulting in the incorrect operation type
used.

This patch fixes the issue by moving both kuduSession and operationType
to be local. Additionaly some minor code cleanup was included.

An integration test, ITPutKudu, was added and used to manual verify the
logging issue existed and is fixed by this patch. I ran the test using
mvn -Pintegration-tests verify -Dtest=ITPutKudu

Calls to `trigger()` may be called concurrently from different threads,
however the PutKudu processor is storing the `kuduSession`
in a class level field. This can result in the logging issue reported in
NIFI-6895 and likely other unusual anomolies including performace
issues depending on the processor configuration.

Additionally the `operationType` was also stored in a class level field
and could be set concurrently resulting in the incorrect operation type
used.

This patch fixes the issue by moving both kuduSession and operationType
to be local. Additionaly some minor code cleanup was included.

An integration test, ITPutKudu, was added and used to manual verify the
logging issue existed and is fixed by this patch. I ran the test using
`mvn -Pintegration-tests verify -Dtest=ITPutKudu`
@pvillard31
Copy link
Contributor

Hey @granthenke - thanks for the PR, I'm going to manually fix the checkstyle issue and merge. Does it also solve the memory leak reported in NIFI-6908?

@asfgit asfgit closed this in 1898ad4 Dec 3, 2019
@granthenke
Copy link
Member Author

granthenke commented Dec 3, 2019

@pvillard31 I think it does, though I haven't had a chance to validate.
I will work on a few other Kudu related changes and validate that it fixes it over the next few days.

@granthenke
Copy link
Member Author

Is there a 1.10.x branch to backport into? This should definitely be included in the next maintenance release.

@pvillard31
Copy link
Contributor

Sounds good. We usually backport bug fixes to a maintenance branch only when we decide to do a minor release via a thread on the mailing lists.

@DaveCLowe
Copy link

@pvillard31 also experiencing this issue and the memleak https://issues.apache.org/jira/browse/NIFI-6908. Any chance there will be a minor release shortly to address? Thanks

@pvillard31
Copy link
Contributor

@DaveCLowe - there is an ongoing discussion on the mailing list to release Apache NiFi 1.11.0 very soon, I don't think there will be a maintenance release for the 1.10 branch.

patricker pushed a commit to patricker/nifi that referenced this pull request Jan 22, 2020
Calls to `trigger()` may be called concurrently from different threads,
however the PutKudu processor is storing the `kuduSession`
in a class level field. This can result in the logging issue reported in
NIFI-6895 and likely other unusual anomolies including performace
issues depending on the processor configuration.

Additionally the `operationType` was also stored in a class level field
and could be set concurrently resulting in the incorrect operation type
used.

This patch fixes the issue by moving both kuduSession and operationType
to be local. Additionaly some minor code cleanup was included.

An integration test, ITPutKudu, was added and used to manual verify the
logging issue existed and is fixed by this patch. I ran the test using
`mvn -Pintegration-tests verify -Dtest=ITPutKudu`

Signed-off-by: Pierre Villard <pierre.villard.fr@gmail.com>

This closes apache#3910.
natural pushed a commit to natural/nifi that referenced this pull request Feb 1, 2020
Calls to `trigger()` may be called concurrently from different threads,
however the PutKudu processor is storing the `kuduSession`
in a class level field. This can result in the logging issue reported in
NIFI-6895 and likely other unusual anomolies including performace
issues depending on the processor configuration.

Additionally the `operationType` was also stored in a class level field
and could be set concurrently resulting in the incorrect operation type
used.

This patch fixes the issue by moving both kuduSession and operationType
to be local. Additionaly some minor code cleanup was included.

An integration test, ITPutKudu, was added and used to manual verify the
logging issue existed and is fixed by this patch. I ran the test using
`mvn -Pintegration-tests verify -Dtest=ITPutKudu`

Signed-off-by: Pierre Villard <pierre.villard.fr@gmail.com>

This closes apache#3910.
ekovacs pushed a commit to ekovacs/nifi-apache that referenced this pull request Feb 24, 2020
Calls to `trigger()` may be called concurrently from different threads,
however the PutKudu processor is storing the `kuduSession`
in a class level field. This can result in the logging issue reported in
NIFI-6895 and likely other unusual anomolies including performace
issues depending on the processor configuration.

Additionally the `operationType` was also stored in a class level field
and could be set concurrently resulting in the incorrect operation type
used.

This patch fixes the issue by moving both kuduSession and operationType
to be local. Additionaly some minor code cleanup was included.

An integration test, ITPutKudu, was added and used to manual verify the
logging issue existed and is fixed by this patch. I ran the test using
`mvn -Pintegration-tests verify -Dtest=ITPutKudu`

Signed-off-by: Pierre Villard <pierre.villard.fr@gmail.com>

This closes apache#3910.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants