You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@james.apache.org by "chibenwa (via GitHub)" <gi...@apache.org> on 2024/11/04 10:47:24 UTC
[PR] [FIX] No longer use recursivity in ReactiveThrottler::onRequestDone [james-project]
chibenwa opened a new pull request, #2486:
URL: https://github.com/apache/james-project/pull/2486
The ReactiveThrottler can be (subscriber direct) recursive, leading to StackOverflowError that could cause unpredictable failure.
Very rare yet spotted in production twice in a row, effectively killing the IMAP service:
```
java.lang.StackOverflowError: null
at java.base/java.text.DecimalFormat.format(Unknown Source)
at java.base/java.text.DecimalFormat.format(Unknown Source)
at java.base/java.text.SimpleDateFormat.zeroPaddingNumber(Unknown Source)
at java.base/java.text.SimpleDateFormat.subFormat(Unknown Source)
at java.base/java.text.SimpleDateFormat.format(Unknown Source)
at java.base/java.text.SimpleDateFormat.format(Unknown Source)
at java.base/java.text.DateFormat.format(Unknown Source)
at ch.qos.logback.contrib.json.JsonLayoutBase.format(Unknown Source)
at ch.qos.logback.contrib.json.JsonLayoutBase.formatTimestamp(Unknown Source)
at ch.qos.logback.contrib.json.JsonLayoutBase.addTimestamp(Unknown Source)
at ch.qos.logback.contrib.json.classic.JsonLayout.toJsonMap(Unknown Source)
at ch.qos.logback.contrib.json.classic.JsonLayout.toJsonMap(Unknown Source)
at ch.qos.logback.contrib.json.JsonLayoutBase.doLayout(Unknown Source)
at ch.qos.logback.core.encoder.LayoutWrappingEncoder.encode(LayoutWrappingEncoder.java:114)
at ch.qos.logback.core.OutputStreamAppender.writeOut(OutputStreamAppender.java:192)
at ch.qos.logback.core.OutputStreamAppender.subAppend(OutputStreamAppender.java:237)
at ch.qos.logback.core.OutputStreamAppender.append(OutputStreamAppender.java:102)
at ch.qos.logback.core.UnsynchronizedAppenderBase.doAppend(UnsynchronizedAppenderBase.java:85)
at ch.qos.logback.core.spi.AppenderAttachableImpl.appendLoopOnAppenders(AppenderAttachableImpl.java:51)
at ch.qos.logback.classic.Logger.appendLoopOnAppenders(Logger.java:272)
at ch.qos.logback.classic.Logger.callAppenders(Logger.java:259)
at ch.qos.logback.classic.Logger.buildLoggingEventAndAppend(Logger.java:426)
at ch.qos.logback.classic.Logger.filterAndLog_0_Or3Plus(Logger.java:386)
at ch.qos.logback.classic.Logger.warn(Logger.java:697)
at reactor.util.Loggers$Slf4JLogger.warn(Loggers.java:304)
at reactor.core.Exceptions.throwIfFatal(Exceptions.java:513)
at reactor.core.publisher.Operators.onOperatorError(Operators.java:756)
at reactor.core.publisher.Operators.onOperatorError(Operators.java:734)
at reactor.core.publisher.Operators.onOperatorError(Operators.java:716)
at reactor.core.publisher.Operators.reportThrowInSubscribe(Operators.java:232)
at reactor.core.publisher.Mono.subscribe(Mono.java:4571)
at reactor.core.publisher.Mono.subscribeWith(Mono.java:4634)
at reactor.core.publisher.Mono.subscribe(Mono.java:4395)
at org.apache.james.imapserver.netty.ReactiveThrottler.onRequestDone(ReactiveThrottler.java:129)
at org.apache.james.imapserver.netty.ReactiveThrottler.lambda$onRequestDone$5(ReactiveThrottler.java:127)
at reactor.core.publisher.FluxDoFinally$DoFinallySubscriber.runFinally(FluxDoFinally.java:146)
at reactor.core.publisher.FluxDoFinally$DoFinallySubscriber.onComplete(FluxDoFinally.java:129)
at reactor.core.publisher.MonoIgnoreThen$ThenIgnoreMain.onComplete(MonoIgnoreThen.java:210)
at reactor.core.publisher.MonoIgnoreThen$ThenIgnoreMain.subscribeNext(MonoIgnoreThen.java:239)
at reactor.core.publisher.MonoIgnoreThen$ThenIgnoreMain.onComplete(MonoIgnoreThen.java:204)
at reactor.core.publisher.FluxContextWrite$ContextWriteSubscriber.onComplete(FluxContextWrite.java:126)
at reactor.core.publisher.FluxDoOnEach$DoOnEachSubscriber.onComplete(FluxDoOnEach.java:223)
at reactor.core.publisher.FluxContextWrite$ContextWriteSubscriber.onComplete(FluxContextWrite.java:126)
at reactor.core.publisher.MonoUsing$MonoUsingSubscriber.onComplete(MonoUsing.java:283)
at reactor.core.publisher.Operators$MultiSubscriptionSubscriber.onComplete(Operators.java:2231)
at reactor.core.publisher.Operators$MultiSubscriptionSubscriber.onComplete(Operators.java:2231)
at reactor.core.publisher.Operators$MultiSubscriptionSubscriber.onComplete(Operators.java:2231)
at reactor.core.publisher.MonoIgnoreThen$ThenIgnoreMain.onComplete(MonoIgnoreThen.java:210)
at reactor.core.publisher.MonoIgnoreThen$ThenIgnoreMain.subscribeNext(MonoIgnoreThen.java:239)
at reactor.core.publisher.MonoIgnoreThen.subscribe(MonoIgnoreThen.java:51)
at reactor.core.publisher.InternalMonoOperator.subscribe(InternalMonoOperator.java:76)
at reactor.core.publisher.MonoUsing.subscribe(MonoUsing.java:109)
...
```
--
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: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
Re: [PR] [FIX] No longer use recursivity in ReactiveThrottler::onRequestDone [james-project]
Posted by "chibenwa (via GitHub)" <gi...@apache.org>.
chibenwa commented on PR #2486:
URL: https://github.com/apache/james-project/pull/2486#issuecomment-2478140084
Running for a week in production, it did solve the stak overflow error without compromising throttling.
Also it ensure all IMAP request processing starts from the eventloop which wasn't the case before.
--
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: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
Re: [PR] [FIX] No longer use recursivity in ReactiveThrottler::onRequestDone [james-project]
Posted by "chibenwa (via GitHub)" <gi...@apache.org>.
chibenwa commented on PR #2486:
URL: https://github.com/apache/james-project/pull/2486#issuecomment-2457568663
Don't merge it does not work.
```
@Test
void test() {
Mono<Void> m = Mono.delay(Duration.ofMillis(1)).then();
for (int i = 0; i < 2000; i++) {
int j = i;
m = m.doFinally(any -> System.out.println(j));
}
m.block();
}
```
--
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: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
Re: [PR] [FIX] No longer use recursivity in ReactiveThrottler::onRequestDone [james-project]
Posted by "chibenwa (via GitHub)" <gi...@apache.org>.
chibenwa merged PR #2486:
URL: https://github.com/apache/james-project/pull/2486
--
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: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
Re: [PR] [FIX] No longer use recursivity in ReactiveThrottler::onRequestDone [james-project]
Posted by "chibenwa (via GitHub)" <gi...@apache.org>.
chibenwa commented on PR #2486:
URL: https://github.com/apache/james-project/pull/2486#issuecomment-2468263817
> Can we squash the fixups?
Nope, let's be sure we fixes some real-world IMAP issue and that this changeset is prod backed.
--
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: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
Re: [PR] [FIX] No longer use recursivity in ReactiveThrottler::onRequestDone [james-project]
Posted by "Arsnael (via GitHub)" <gi...@apache.org>.
Arsnael commented on PR #2486:
URL: https://github.com/apache/james-project/pull/2486#issuecomment-2467433962
Can we squash the fixups?
--
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: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
Re: [PR] [FIX] No longer use recursivity in ReactiveThrottler::onRequestDone [james-project]
Posted by "chibenwa (via GitHub)" <gi...@apache.org>.
chibenwa commented on PR #2486:
URL: https://github.com/apache/james-project/pull/2486#issuecomment-2457737941
```
private Sinks.Many<Integer> objectMany;
private CountDownLatch countDownLatch;
@Test
void test() throws Exception {
countDownLatch = new CountDownLatch(1);
objectMany = Sinks.many().multicast().onBackpressureBuffer();
objectMany.asFlux()
.subscribeOn(Schedulers.parallel())
.subscribe(i -> Mono.delay(Duration.ofMillis(1))
.doFinally(any -> betise(i))
.subscribe());
objectMany.emitNext(1, Sinks.EmitFailureHandler.FAIL_FAST);
countDownLatch.await();
}
void betise(int i) {
if (i < 20000) {
System.out.println(i);
objectMany.emitNext(i + 1, Sinks.EmitFailureHandler.FAIL_FAST);
} else {
countDownLatch.countDown();
}
}
```
Using signaling technics apparently do avoid the effect mentioned in https://github.com/apache/james-project/pull/2486#issuecomment-2457568663
--
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: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org