Skip to content

Commit ed283be

Browse files
committed
Address review feedback for xdebug transform fix
- Update comment to explain 'why' (prevents tight loop that starves other transactions) instead of 'what' the code does - Change verify_no_loop.sh to fail the test when consumed_count != 2 instead of just warning
1 parent dd71997 commit ed283be

2 files changed

Lines changed: 5 additions & 4 deletions

File tree

plugins/xdebug/xdebug_transforms.cc

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -297,9 +297,9 @@ body_transform(TSCont contp, TSEvent event, void * /* edata ATS_UNUSED */)
297297

298298
if (TSVIONTodoGet(src_vio) > 0) {
299299
if (towrite > 0) {
300-
// Only reenable and notify upstream if we consumed data.
301-
// If no data was available, just return and wait for the
302-
// upstream to call us again when more data arrives.
300+
// Only reenable when we consumed data. If we reenable and call
301+
// WRITE_READY when towrite is 0 (no data available yet), we create
302+
// a tight loop that starves other transactions and causes high CPU.
303303
TSVIOReenable(data->output_vio);
304304
TSContCall(TSVIOContGet(src_vio), TS_EVENT_VCONN_WRITE_READY, src_vio);
305305
}

tests/gold_tests/pluginTest/xdebug/x_probe_full_json_slow_origin/verify_no_loop.sh

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,8 @@ consumed_count=$(grep -c "consumed.*bytes" "$LOGFILE" 2>/dev/null || echo "0")
5656
echo " 'consumed' lines: $consumed_count (expected: 2)"
5757

5858
if [[ $consumed_count -ne 2 ]]; then
59-
echo "WARNING: Expected 2 consumed lines but found $consumed_count"
59+
echo "FAIL: Expected 2 consumed lines but found $consumed_count"
60+
exit 1
6061
fi
6162

6263
echo "PASS: Transform completed normally"

0 commit comments

Comments
 (0)