Skip to content

Commit 77b892e

Browse files
committed
Remove backpressure debug warning
In cases where backpressure signals are being pathologically ignored, the additional warning becomes too spammy to be useful. Given that it's often out of the users control (as in the case of a worker using next.js which suffers from this), the warning becomes unhelpful additional noise. Just remove it.
1 parent 387f39d commit 77b892e

File tree

4 files changed

+0
-52
lines changed

4 files changed

+0
-52
lines changed

src/workerd/api/streams/internal.c++

Lines changed: 0 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -980,29 +980,6 @@ void WritableStreamInternalController::increaseCurrentWriteBufferSize(
980980
KJ_IF_SOME(highWaterMark, maybeHighWaterMark) {
981981
int64_t amount = highWaterMark - currentWriteBufferSize;
982982
updateBackpressure(js, amount <= 0);
983-
// If the current buffer size is greater than or equal to double the high water mark,
984-
// let's emit a warning about excessive backpressure.
985-
// TODO(later): For the standard stream, we use a variable multiplier if the highWaterMark
986-
// is < 10 because the default high water mark is 1 and we don't want to emit the warning
987-
// too often. For internal streams, tho, there is no default high water mark and the user
988-
// would have to provide one... and since these are always bytes it would make sense
989-
// for the user to specify a larger value here in the typical case... so I decided to go with
990-
// the fixed 2x multiplier. However, I can make this variable too if folks feel the consistency
991-
// is important.
992-
if (warnAboutExcessiveBackpressure && (currentWriteBufferSize >= 2 * highWaterMark)) {
993-
excessiveBackpressureWarningCount++;
994-
auto warning = kj::str("A WritableStream is experiencing excessive backpressure. "
995-
"The current write buffer size is ",
996-
currentWriteBufferSize,
997-
" bytes, which is greater than or equal to double the high water mark "
998-
"of ",
999-
highWaterMark,
1000-
" bytes. Streams that consistently exceed the "
1001-
"configured high water mark may cause excessive memory usage. ",
1002-
"(Count ", excessiveBackpressureWarningCount, ")");
1003-
js.logWarning(warning);
1004-
warnAboutExcessiveBackpressure = false;
1005-
}
1006983
}
1007984
}
1008985

@@ -1028,7 +1005,6 @@ void WritableStreamInternalController::updateBackpressure(jsg::Lock& js, bool ba
10281005
}
10291006

10301007
// When backpressure is updated and is false, we resolve the ready promise on the writer
1031-
warnAboutExcessiveBackpressure = true;
10321008
maybeResolvePromise(js, writerLock.getReadyFulfiller());
10331009
}
10341010
}

src/workerd/api/streams/internal.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -276,8 +276,6 @@ class WritableStreamInternalController: public WritableStreamController {
276276
kj::Maybe<kj::Own<PendingAbort>> maybePendingAbort;
277277

278278
uint64_t currentWriteBufferSize = 0;
279-
bool warnAboutExcessiveBackpressure = true;
280-
size_t excessiveBackpressureWarningCount = 0;
281279

282280
// The highWaterMark is the total amount of data currently buffered in
283281
// the controller waiting to be flushed out to the underlying WritableStreamSink.

src/workerd/api/streams/standard.c++

Lines changed: 0 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1492,30 +1492,6 @@ void WritableImpl<Self>::updateBackpressure(jsg::Lock& js) {
14921492
KJ_ASSERT(!isCloseQueuedOrInFlight());
14931493
bool bp = getDesiredSize() < 0;
14941494

1495-
// We use a variable multiplier here in order to prevent the warning from being too
1496-
// spammy in the default case. The default high water mark for a standard writable stream
1497-
// is 1, which means we'd end up emitting a warning every time the buffer size is greater
1498-
// than 2, which is not very helpful. Instead, for any highWaterMark < 10, we'll configure
1499-
// a multiplier of 10, and for any highWaterMark >= 10, we'll configure a multiplier of 2.
1500-
// This is fairly arbitrary and may need to be tuned further.
1501-
int warningMultiplier = highWaterMark <= 10 ? 10 : 2;
1502-
1503-
if (flags.warnAboutExcessiveBackpressure &&
1504-
(amountBuffered >= warningMultiplier * highWaterMark)) {
1505-
excessiveBackpressureWarningCount++;
1506-
auto warning = kj::str("A WritableStream is experiencing excessive backpressure. "
1507-
"The current write buffer size is ",
1508-
amountBuffered, ", which is greater than or equal to ", warningMultiplier,
1509-
" times the high water mark of ", highWaterMark,
1510-
". Streams that consistently exceed the configured high water ",
1511-
"mark may cause excessive memory usage. ", "(Count ", excessiveBackpressureWarningCount,
1512-
")");
1513-
js.logWarning(warning);
1514-
flags.warnAboutExcessiveBackpressure = false;
1515-
}
1516-
1517-
if (!bp) flags.warnAboutExcessiveBackpressure = true;
1518-
15191495
if (bp != flags.backpressure) {
15201496
flags.backpressure = bp;
15211497
KJ_IF_SOME(owner, tryGetOwner()) {

src/workerd/api/streams/standard.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -374,7 +374,6 @@ class WritableImpl {
374374

375375
size_t highWaterMark = 1;
376376
size_t amountBuffered = 0;
377-
size_t excessiveBackpressureWarningCount = 0;
378377

379378
// `writeRequests` is often going to be empty in common usage patterns, in which case std::list
380379
// is more memory efficient than a std::deque, for example.
@@ -389,7 +388,6 @@ class WritableImpl {
389388
uint8_t started : 1 = 0;
390389
uint8_t starting : 1 = 0;
391390
uint8_t backpressure : 1 = 0;
392-
uint8_t warnAboutExcessiveBackpressure : 1 = 1;
393391
};
394392
Flags flags{};
395393

0 commit comments

Comments
 (0)