ijokarumawak commented on issue #3276: NIFI-5953 Manage GetTwitter connection retries on '420 Enhance Your C… URL: https://github.com/apache/nifi/pull/3276#issuecomment-475539434 @kourge-ch Thanks for your contribution and detailed explanation about the issue! Sorry for taking time to give you review feedback. I looked at the hbc client source code. I believe this part is the one you mentioned on the JIRA description that hbc client handles exponential retry. Am I correct? https://github.com/twitter/hbc/blob/ce61517be1e726fb4f5809786af86e14a559a4d7/hbc-core/src/main/java/com/twitter/hbc/httpclient/ClientBase.java#L220-L232 If so, the client code handles all 4xx HTTP status codes the same. Probably we shouldn't treat 420 special. Instead, we should avoid reconnecting for all HTTP_ERROR events including 500. To be specific, what the hbc client does for 4xx and 5xx status codes is that reconnectManager just wait for a while exponentially to make another request. For 4xx, the client exits with STOPPED_BY_ERROR after it receives 4xx more than configured retry count (defaults to 5). That makes sense because 4xx represents errors due to bad client request. Keep sending the same request indefinitely will not recover the error. For 5xx, there's no retry limit. That makes sense, because 5xx represents server side errors. Retrying may recover the error. In contrast, what `client.reconnect` does is stop reading the successful response and reconnect the API again. https://github.com/twitter/hbc/blob/ce61517be1e726fb4f5809786af86e14a559a4d7/hbc-core/src/main/java/com/twitter/hbc/httpclient/ClientBase.java#L133 From above, I think we should not call `client.reconnect` for any HTTP_ERROR cases.
---------------------------------------------------------------- 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: [email protected] With regards, Apache Git Services
