Merge pull request #5356 from cloudflare/jasnell/remove-unhelpful-backpressure-warning

Author: jasnell

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

@@ -980,29 +980,6 @@ void WritableStreamInternalController::increaseCurrentWriteBufferSize(
   KJ_IF_SOME(highWaterMark, maybeHighWaterMark) {
     int64_t amount = highWaterMark - currentWriteBufferSize;
     updateBackpressure(js, amount <= 0);
-    // If the current buffer size is greater than or equal to double the high water mark,
-    // let's emit a warning about excessive backpressure.
-    // TODO(later): For the standard stream, we use a variable multiplier if the highWaterMark
-    // is < 10 because the default high water mark is 1 and we don't want to emit the warning
-    // too often. For internal streams, tho, there is no default high water mark and the user
-    // would have to provide one... and since these are always bytes it would make sense
-    // for the user to specify a larger value here in the typical case... so I decided to go with
-    // the fixed 2x multiplier. However, I can make this variable too if folks feel the consistency
-    // is important.
-    if (warnAboutExcessiveBackpressure && (currentWriteBufferSize >= 2 * highWaterMark)) {
-      excessiveBackpressureWarningCount++;
-      auto warning = kj::str("A WritableStream is experiencing excessive backpressure. "
-                             "The current write buffer size is ",
-          currentWriteBufferSize,
-          " bytes, which is greater than or equal to double the high water mark "
-          "of ",
-          highWaterMark,
-          " bytes. Streams that consistently exceed the "
-          "configured high water mark may cause excessive memory usage. ",
-          "(Count ", excessiveBackpressureWarningCount, ")");
-      js.logWarning(warning);
-      warnAboutExcessiveBackpressure = false;
-    }
   }
 }
 
@@ -1028,7 +1005,6 @@ void WritableStreamInternalController::updateBackpressure(jsg::Lock& js, bool ba
     }
 
     // When backpressure is updated and is false, we resolve the ready promise on the writer
-    warnAboutExcessiveBackpressure = true;
     maybeResolvePromise(js, writerLock.getReadyFulfiller());
   }
 }

src/workerd/api/streams/internal.h

@@ -276,8 +276,6 @@ class WritableStreamInternalController: public WritableStreamController {
   kj::Maybe<kj::Own<PendingAbort>> maybePendingAbort;
 
   uint64_t currentWriteBufferSize = 0;
-  bool warnAboutExcessiveBackpressure = true;
-  size_t excessiveBackpressureWarningCount = 0;
 
   // The highWaterMark is the total amount of data currently buffered in
   // the controller waiting to be flushed out to the underlying WritableStreamSink.

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

@@ -1492,30 +1492,6 @@ void WritableImpl<Self>::updateBackpressure(jsg::Lock& js) {
   KJ_ASSERT(!isCloseQueuedOrInFlight());
   bool bp = getDesiredSize() < 0;
 
-  // We use a variable multiplier here in order to prevent the warning from being too
-  // spammy in the default case. The default high water mark for a standard writable stream
-  // is 1, which means we'd end up emitting a warning every time the buffer size is greater
-  // than 2, which is not very helpful. Instead, for any highWaterMark < 10, we'll configure
-  // a multiplier of 10, and for any highWaterMark >= 10, we'll configure a multiplier of 2.
-  // This is fairly arbitrary and may need to be tuned further.
-  int warningMultiplier = highWaterMark <= 10 ? 10 : 2;
-
-  if (flags.warnAboutExcessiveBackpressure &&
-      (amountBuffered >= warningMultiplier * highWaterMark)) {
-    excessiveBackpressureWarningCount++;
-    auto warning = kj::str("A WritableStream is experiencing excessive backpressure. "
-                           "The current write buffer size is ",
-        amountBuffered, ", which is greater than or equal to ", warningMultiplier,
-        " times the high water mark of ", highWaterMark,
-        ". Streams that consistently exceed the configured high water ",
-        "mark may cause excessive memory usage. ", "(Count ", excessiveBackpressureWarningCount,
-        ")");
-    js.logWarning(warning);
-    flags.warnAboutExcessiveBackpressure = false;
-  }
-
-  if (!bp) flags.warnAboutExcessiveBackpressure = true;
-
   if (bp != flags.backpressure) {
     flags.backpressure = bp;
     KJ_IF_SOME(owner, tryGetOwner()) {

src/workerd/api/streams/standard.h

@@ -374,7 +374,6 @@ class WritableImpl {
 
   size_t highWaterMark = 1;
   size_t amountBuffered = 0;
-  size_t excessiveBackpressureWarningCount = 0;
 
   // `writeRequests` is often going to be empty in common usage patterns, in which case std::list
   // is more memory efficient than a std::deque, for example.
@@ -389,7 +388,6 @@ class WritableImpl {
     uint8_t started : 1 = 0;
     uint8_t starting : 1 = 0;
     uint8_t backpressure : 1 = 0;
-    uint8_t warnAboutExcessiveBackpressure : 1 = 1;
   };
   Flags flags{};