jrushford commented on a change in pull request #8098:
URL: https://github.com/apache/trafficserver/pull/8098#discussion_r678536765
##########
File path: proxy/http/remap/NextHopConsistentHash.cc
##########
@@ -326,6 +326,7 @@ NextHopConsistentHash::findNextHop(TSHttpTxn txnp, void
*ih, time_t now)
// check if the host is retryable. It's retryable if the retry window
has elapsed
_now == 0 ? _now = time(nullptr) : _now = now;
if ((pRec->failedAt.load() + retry_time) <
static_cast<unsigned>(_now)) {
+ NH_Debug(NH_DEBUG_TAG, "[%" PRIu64 "] next hop %s, retriers: %d",
sm_id, pRec->hostname.c_str(), pRec->retriers.load());
Review comment:
With that said, I like how you've abstracted the retriers in the other
PR.
##########
File path: proxy/http/remap/NextHopConsistentHash.cc
##########
@@ -326,6 +326,7 @@ NextHopConsistentHash::findNextHop(TSHttpTxn txnp, void
*ih, time_t now)
// check if the host is retryable. It's retryable if the retry window
has elapsed
_now == 0 ? _now = time(nullptr) : _now = now;
if ((pRec->failedAt.load() + retry_time) <
static_cast<unsigned>(_now)) {
+ NH_Debug(NH_DEBUG_TAG, "[%" PRIu64 "] next hop %s, retriers: %d",
sm_id, pRec->hostname.c_str(), pRec->retriers.load());
Review comment:
I think that was the idea with the else clause @ywkaras, to negate the
increment on line 330 if they aren't retriers. Am I missing something here?
##########
File path: proxy/http/remap/NextHopConsistentHash.cc
##########
@@ -326,6 +326,7 @@ NextHopConsistentHash::findNextHop(TSHttpTxn txnp, void
*ih, time_t now)
// check if the host is retryable. It's retryable if the retry window
has elapsed
_now == 0 ? _now = time(nullptr) : _now = now;
if ((pRec->failedAt.load() + retry_time) <
static_cast<unsigned>(_now)) {
+ NH_Debug(NH_DEBUG_TAG, "[%" PRIu64 "] next hop %s, retriers: %d",
sm_id, pRec->hostname.c_str(), pRec->retriers.load());
Review comment:
That was the idea of the else clause, to back out the increment if the
transaction on that core wasn't going to be a retrier. Am I missing something
@ywkaras? I do like how you've abstracted this in the other PR though.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
