Skip to content

Commit

Permalink
[SDCISA-16293] Self-review. Cleanup.
Browse files Browse the repository at this point in the history
  • Loading branch information
hiddenalpha committed Jun 24, 2024
1 parent 670be9a commit 3578782
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 22 deletions.
29 changes: 10 additions & 19 deletions src/main/java/org/swisspush/redisques/RedisQues.java
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
import io.vertx.core.eventbus.EventBus;
import io.vertx.core.eventbus.Message;
import io.vertx.core.eventbus.MessageConsumer;
import io.vertx.core.impl.NoStackTraceThrowable;
import io.vertx.core.json.JsonArray;
import io.vertx.core.json.JsonObject;
import io.vertx.redis.client.Command;
Expand Down Expand Up @@ -1259,29 +1258,21 @@ private Future<Void> checkQueues() {
return ctx.iter.hasNext();
}
@Override public boolean onError(Throwable ex, Void ctx_) {
for (Throwable waitQueFullEx = ex; waitQueFullEx != null; waitQueFullEx = waitQueFullEx.getCause()) {
if (!(waitQueFullEx instanceof NoStackTraceThrowable)) continue;
if (!"Redis waiting queue is full".equals(waitQueFullEx.getMessage())) continue;
// Trying to continue (aka return true) makes no sense as our redis
// client is out of resources and continuing just will make load
// situation worse. So we abort in the case of this special exception.
makeGcLifeEasier();
p.fail(exceptionFactory.newException(ex));
return false;
}
log.warn("TODO error handling", exceptionFactory.newException(ex));
// Still not sure if it is a good idea to ignore errors and try to respond
// with half-baked results. But will keep it this way because old code [1]
// did without any comment that would have explained why.
// Old code [1] for whatever reason did just continue after an error.
// Nevertheless, I'll now change it to abort (aka return false). Due to
// the absence of comments I have no idea what those half-baked results
// where used for. See also [2] about this decision.
// [1]: https://github.com/swisspost/vertx-redisques/blob/v3.0.33/src/main/java/org/swisspush/redisques/RedisQues.java
return true; // true, aka keep going with other queues.
// [2]: https://github.com/swisspost/vertx-redisques/issues/192
reduceGcGraphTraversalEffort();
p.fail(exceptionFactory.newException(ex));
return false;
}
@Override public void onDone(Void ctx_) {
makeGcLifeEasier();
reduceGcGraphTraversalEffort();
p.complete(); // Mark this composition step as completed.
}
private void makeGcLifeEasier() {
// No longer used, so reduce GC graph traversal effort.
private void reduceGcGraphTraversalEffort() {
ctx.redisAPI = null;
ctx.counter = null;
ctx.iter = null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ public void logSomewhen(Ctx ctx) {
mostRecentCtx = ctx;
count += 1;
durationSincePrevPublishMs = now - prevPublishEpchMs;
if (durationSincePrevPublishMs > minDelayMs) {
if (durationSincePrevPublishMs >= minDelayMs) {
isPublish = true;
prevPublishEpchMs = now;
countLocalCpy = count;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -184,8 +184,10 @@ private <Ctx> void onOneDone(Request<Ctx> req, Throwable ex) {
req.lock.lock(); // Need our lock back.
req.isFatalError = isFatalError;
// Need to release our token now. As we won't do it later anymore.
req.numTokensAvailForOurself -= 1;
req.limit.release();
if (isFatalError) {
req.numTokensAvailForOurself -= 1;
req.limit.release();
}
}
} finally {
req.lock.unlock();
Expand Down

0 comments on commit 3578782

Please sign in to comment.