You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by GitBox <gi...@apache.org> on 2020/04/20 23:09:35 UTC
[GitHub] [nifi] taftster opened a new pull request #4221: NIFI-6394 - frontend queue/connection size limit
taftster opened a new pull request #4221:
URL: https://github.com/apache/nifi/pull/4221
This PR fundamentally removes the hard-coded number of flowfiles in the listing returned by StandardConnectionDAO.java, adding a new parameter called 'maxResults' the parent interface. This value can then be consulted and used when calling the backend for the queue listing. The current hard-coded value of '100' can be limiting in certain use cases, for example, when the queue has a backlog of > 100 flowfiles and the dataflow manager needs to interrogate the 101+ flowfile.
The maxResults parameter is exposed as part of the REST API, so that calling programs (like the UI) can control the number of displayed flowfiles. By default, the listing size will remain at 100, but a UI option is added to "view all" flowfiles in the queue. Additionally, the parameter can be controlled by the REST API for custom clients.
This PR is broken into two commits; one for the backend changes, one for the UI changes. These can be considered separately or can be easily squashed together as desired.
This features closes NIFI-6394
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] pvillard31 commented on pull request #4221: NIFI-6394 - frontend queue/connection size limit
Posted by GitBox <gi...@apache.org>.
pvillard31 commented on pull request #4221:
URL: https://github.com/apache/nifi/pull/4221#issuecomment-662428690
@taftster - you made a comment about making a fix but I didn't see any new commit since then. Am I overlooking something?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] taftster commented on pull request #4221: NIFI-6394 - frontend queue/connection size limit
Posted by GitBox <gi...@apache.org>.
taftster commented on pull request #4221:
URL: https://github.com/apache/nifi/pull/4221#issuecomment-702239736
OK @anaylor - I'll follow your lead. If you come up with a better design, then I can grab your commits and add them here. Or you can grab my commits and we'll merge your branch. Either way, whatever is easiest.
And if you end up taking on the comment above (to deprecate the original api method, not remove it), that would be great too. But if you don't get to that, I will do it.
In short, I will follow your lead here. Just let me know how to support.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] anaylor edited a comment on pull request #4221: NIFI-6394 - frontend queue/connection size limit
Posted by GitBox <gi...@apache.org>.
anaylor edited a comment on pull request #4221:
URL: https://github.com/apache/nifi/pull/4221#issuecomment-662424287
Any updates on this getting merged?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] jfrazee commented on a change in pull request #4221: NIFI-6394 - frontend queue/connection size limit
Posted by GitBox <gi...@apache.org>.
jfrazee commented on a change in pull request #4221:
URL: https://github.com/apache/nifi/pull/4221#discussion_r442902452
##########
File path: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/NiFiServiceFacade.java
##########
@@ -767,9 +767,10 @@
*
* @param connectionId The ID of the connection
* @param listingRequestId The ID of the listing request
+ * @param maxResults The maximum number of flowfile summary objects to return
* @return The ListingRequest
*/
- ListingRequestDTO createFlowFileListingRequest(String connectionId, String listingRequestId);
Review comment:
Have been meaning to mention this. Since this is public in an API package, I think this needs to be kept around and marked @Deprecated instead of deleted outright. Same comment on some of the other methods below.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] taftster commented on a change in pull request #4221: NIFI-6394 - frontend queue/connection size limit
Posted by GitBox <gi...@apache.org>.
taftster commented on a change in pull request #4221:
URL: https://github.com/apache/nifi/pull/4221#discussion_r443000194
##########
File path: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/NiFiServiceFacade.java
##########
@@ -767,9 +767,10 @@
*
* @param connectionId The ID of the connection
* @param listingRequestId The ID of the listing request
+ * @param maxResults The maximum number of flowfile summary objects to return
* @return The ListingRequest
*/
- ListingRequestDTO createFlowFileListingRequest(String connectionId, String listingRequestId);
Review comment:
Yup, totally. +1 I should have thought of this, not sure why I didn't. Good catch, thank you. Will fix.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] taftster commented on pull request #4221: NIFI-6394 - frontend queue/connection size limit
Posted by GitBox <gi...@apache.org>.
taftster commented on pull request #4221:
URL: https://github.com/apache/nifi/pull/4221#issuecomment-726153715
Whoops, @anaylor OK I see what's happened. Let's move forward with your other pull request and close this one. Thanks for moving forward!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] taftster commented on pull request #4221: NIFI-6394 - frontend queue/connection size limit
Posted by GitBox <gi...@apache.org>.
taftster commented on pull request #4221:
URL: https://github.com/apache/nifi/pull/4221#issuecomment-698674405
@anaylor I still owe everyone the refactor that is to deprecate the existing method, not remove it. I just have had to keep putting it off. /sigh
That being said ... if you can:
1. Smash your changes onto this branch - that would be awesome. I'll take them for sure.
2. Do you want to also refactor to deprecate the api method? (e.g. per @jfrazee 's comment)
I don't need credit for this or anything, so if you just want to adopt and own, it's all yours. I'm super exited to see a nice element for this instead of a stupid link.
Will wait to hear from you. Thanks for your contribution!!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] taftster closed pull request #4221: NIFI-6394 - frontend queue/connection size limit
Posted by GitBox <gi...@apache.org>.
taftster closed pull request #4221:
URL: https://github.com/apache/nifi/pull/4221
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] anaylor commented on pull request #4221: NIFI-6394 - frontend queue/connection size limit
Posted by GitBox <gi...@apache.org>.
anaylor commented on pull request #4221:
URL: https://github.com/apache/nifi/pull/4221#issuecomment-698506510
@taftster I developed the front end for this ticket, if you can push this I could through my changes in. Or smash it on here, your call
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] pvillard31 commented on pull request #4221: NIFI-6394 - frontend queue/connection size limit
Posted by GitBox <gi...@apache.org>.
pvillard31 commented on pull request #4221:
URL: https://github.com/apache/nifi/pull/4221#issuecomment-662617545
No rush at all @taftster - if it does not make it into 1.12.0 that's totally fine. (And I don't expect 1.12.0 release work to start right now AFAIK)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] anaylor commented on pull request #4221: NIFI-6394 - frontend queue/connection size limit
Posted by GitBox <gi...@apache.org>.
anaylor commented on pull request #4221:
URL: https://github.com/apache/nifi/pull/4221#issuecomment-698506510
@taftster I developed the front end for this ticket, if you can push this I could through my changes in. Or smash it on here, your call
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] taftster commented on pull request #4221: NIFI-6394 - frontend queue/connection size limit
Posted by GitBox <gi...@apache.org>.
taftster commented on pull request #4221:
URL: https://github.com/apache/nifi/pull/4221#issuecomment-662590695
You're not overlooking. Just busy on my end (as we all are). I can bump this and get it done if it's holding someone up. Likewise as always, if someone wants to run with it, that's fine too.
What timeframe is reasonable? I can probably kick on this again early next week. Does that work?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] anaylor commented on pull request #4221: NIFI-6394 - frontend queue/connection size limit
Posted by GitBox <gi...@apache.org>.
anaylor commented on pull request #4221:
URL: https://github.com/apache/nifi/pull/4221#issuecomment-702209932
Hey @taftster I was planning on pulling in your changes to my existing branch but it would probably make more sense just to smash my changes onto here instead of opening a new PR. Its still just a link for now but intend on making it look nice.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] anaylor commented on pull request #4221: NIFI-6394 - frontend queue/connection size limit
Posted by GitBox <gi...@apache.org>.
anaylor commented on pull request #4221:
URL: https://github.com/apache/nifi/pull/4221#issuecomment-704430785
Hey @taftster I made the changes and cherry picked them to your branch, not authorized to push them however. Can you either give me access or if you have another suggestion would be fine with me.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] taftster commented on pull request #4221: NIFI-6394 - frontend queue/connection size limit
Posted by GitBox <gi...@apache.org>.
taftster commented on pull request #4221:
URL: https://github.com/apache/nifi/pull/4221#issuecomment-698674405
@anaylor I still owe everyone the refactor that is to deprecate the existing method, not remove it. I just have had to keep putting it off. /sigh
That being said ... if you can:
1. Smash your changes onto this branch - that would be awesome. I'll take them for sure.
2. Do you want to also refactor to deprecate the api method? (e.g. per @jfrazee 's comment)
I don't need credit for this or anything, so if you just want to adopt and own, it's all yours. I'm super exited to see a nice element for this instead of a stupid link.
Will wait to hear from you. Thanks for your contribution!!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] taftster commented on pull request #4221: NIFI-6394 - frontend queue/connection size limit
Posted by GitBox <gi...@apache.org>.
taftster commented on pull request #4221:
URL: https://github.com/apache/nifi/pull/4221#issuecomment-726171577
Closed in favor of:
https://github.com/apache/nifi/pull/4641
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] taftster commented on pull request #4221: NIFI-6394 - frontend queue/connection size limit
Posted by GitBox <gi...@apache.org>.
taftster commented on pull request #4221:
URL: https://github.com/apache/nifi/pull/4221#issuecomment-726151846
@anaylor I've been MIA, sorry. Can you push somewhere that I have access to? Just your personal namespace or something? If not, I will sort out permissions and such.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] anaylor commented on pull request #4221: NIFI-6394 - frontend queue/connection size limit
Posted by GitBox <gi...@apache.org>.
anaylor commented on pull request #4221:
URL: https://github.com/apache/nifi/pull/4221#issuecomment-662424287
Any updates on this getting pushed merged?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] jfrazee commented on a change in pull request #4221: NIFI-6394 - frontend queue/connection size limit
Posted by GitBox <gi...@apache.org>.
jfrazee commented on a change in pull request #4221:
URL: https://github.com/apache/nifi/pull/4221#discussion_r442902452
##########
File path: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/NiFiServiceFacade.java
##########
@@ -767,9 +767,10 @@
*
* @param connectionId The ID of the connection
* @param listingRequestId The ID of the listing request
+ * @param maxResults The maximum number of flowfile summary objects to return
* @return The ListingRequest
*/
- ListingRequestDTO createFlowFileListingRequest(String connectionId, String listingRequestId);
Review comment:
Have been meaning to mention this. Since this is public in an API package, I think this needs to be kept around and marked Deprecated instead of deleted outright. Same comment on some of the other methods below.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] joewitt commented on pull request #4221: NIFI-6394 - frontend queue/connection size limit
Posted by GitBox <gi...@apache.org>.
joewitt commented on pull request #4221:
URL: https://github.com/apache/nifi/pull/4221#issuecomment-726153455
It is essential that the solution is considerate regarding memory consumption implications both within a single node and in a cluster case with replicated responses/etc.. 100 is a magic number but it is also a safe/conservative value for how it is all currently implemented. Changing this number to be larger will require full consideration of all implications.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org