brbzull0 opened a new issue, #12933: URL: https://github.com/apache/trafficserver/issues/12933
While doing some other work I found that some records in the nethandler are dead. Asked Claude to investigate and came up with the following ---- Three `proxy.config.net.*` records in `NetHandler` have been dead code since their introduction in 2015 (TS-3313, commit [974e8e3](https://github.com/apache/trafficserver/commit/974e8e3abc9a3f60f516b24302edde5618a3cfc5)). They were never registered in [`RecordsConfig.cc`](https://github.com/apache/trafficserver/blob/40b99c7b57eec10ff8e549a6d6d2b87663804eea/src/records/RecordsConfig.cc), are not documented, and the config values they store are never consumed by any functional code. ## Affected records | Record | Registered in `RecordsConfig.cc`? | Read by any logic? | |--------|-----------------------------------|--------------------| | `proxy.config.net.inactive_threshold_in` | ❌ No | ❌ No | | `proxy.config.net.transaction_no_activity_timeout_in` | ❌ No | ❌ No | | `proxy.config.net.keep_alive_no_activity_timeout_in` | ❌ No | ❌ No | ## What exists today Each record has plumbing that compiles and is structurally reachable, but is dead in practice: 1. **Config struct fields** in [`NetHandler.h` L116-L118](https://github.com/apache/trafficserver/blob/40b99c7b57eec10ff8e549a6d6d2b87663804eea/include/iocore/net/NetHandler.h#L116-L118) — defined but never read by any functional code. 2. **`RecGetRecordInt()` reads** in [`NetHandler.cc` L180-L184](https://github.com/apache/trafficserver/blob/40b99c7b57eec10ff8e549a6d6d2b87663804eea/src/iocore/net/NetHandler.cc#L180-L184) — always return `0` via `value_or(0)` since the records don't exist in [`RecordsConfig.cc`](https://github.com/apache/trafficserver/blob/40b99c7b57eec10ff8e549a6d6d2b87663804eea/src/records/RecordsConfig.cc). 3. **`RecRegisterConfigUpdateCb()` callbacks** in [`NetHandler.cc` L201-L203](https://github.com/apache/trafficserver/blob/40b99c7b57eec10ff8e549a6d6d2b87663804eea/src/iocore/net/NetHandler.cc#L201-L203) — register callbacks on non-existent records, so they can never fire. 4. **Handler branches** in [`NetHandler.cc` L131-L139](https://github.com/apache/trafficserver/blob/40b99c7b57eec10ff8e549a6d6d2b87663804eea/src/iocore/net/NetHandler.cc#L131-L139) — would write to the config fields if the callbacks fired, but they can't (see point 3), and even if they could, no code reads the stored values. 5. **Debug log lines** in [`NetHandler.cc` L210-L214](https://github.com/apache/trafficserver/blob/40b99c7b57eec10ff8e549a6d6d2b87663804eea/src/iocore/net/NetHandler.cc#L210-L214) — always log `0`. ## History - **2015** — TS-3313 ([974e8e3](https://github.com/apache/trafficserver/commit/974e8e3abc9a3f60f516b24302edde5618a3cfc5)): Introduced all three records (with `inactive_threashold_in` typo) in `UnixNet.cc` with `REC_ReadConfigInt32()` and `RecRegisterConfigUpdateCb()`, but **forgot to add them to `RecordsConfig.cc`**. Only `max_connections_in` and `max_connections_active_in` were added to the config table. - **2017** — [69343fe](https://github.com/apache/trafficserver/commit/69343fe99438393caa2f852dea0a62fa3de31f54): Refactored NetHandler init to be static. Fixed typo `inactive_threashold_in` → `inactive_threshold_in`. Carried forward the same unregistered records. - **Present** — Records remain unregistered and unused. ## Not related to the `http` variants There are similarly-named records in the HTTP layer that **are** properly registered and functional: - `proxy.config.http.keep_alive_no_activity_timeout_in` — registered in [`RecordsConfig.cc` L471](https://github.com/apache/trafficserver/blob/40b99c7b57eec10ff8e549a6d6d2b87663804eea/src/records/RecordsConfig.cc#L471), used by [`HttpConfig.cc`](https://github.com/apache/trafficserver/blob/40b99c7b57eec10ff8e549a6d6d2b87663804eea/src/proxy/http/HttpConfig.cc) / [`HttpSM.cc`](https://github.com/apache/trafficserver/blob/40b99c7b57eec10ff8e549a6d6d2b87663804eea/src/proxy/http/HttpSM.cc) - `proxy.config.http.transaction_no_activity_timeout_in` — registered in [`RecordsConfig.cc` L479](https://github.com/apache/trafficserver/blob/40b99c7b57eec10ff8e549a6d6d2b87663804eea/src/records/RecordsConfig.cc#L479), used by [`HttpConfig.cc`](https://github.com/apache/trafficserver/blob/40b99c7b57eec10ff8e549a6d6d2b87663804eea/src/proxy/http/HttpConfig.cc) / [`HttpSM.cc`](https://github.com/apache/trafficserver/blob/40b99c7b57eec10ff8e549a6d6d2b87663804eea/src/proxy/http/HttpSM.cc) These are separate records serving different purposes in the HTTP layer — no replacement or renaming occurred. ## Proposed fix Remove the dead plumbing for all three records: - [ ] Remove the three fields from `Config` struct in [`NetHandler.h`](https://github.com/apache/trafficserver/blob/40b99c7b57eec10ff8e549a6d6d2b87663804eea/include/iocore/net/NetHandler.h#L116-L118) - [ ] Remove the `RecGetRecordInt()` calls in [`NetHandler.cc` `init_for_process()`](https://github.com/apache/trafficserver/blob/40b99c7b57eec10ff8e549a6d6d2b87663804eea/src/iocore/net/NetHandler.cc#L180-L184) - [ ] Remove the `RecRegisterConfigUpdateCb()` calls in [`NetHandler.cc` `init_for_process()`](https://github.com/apache/trafficserver/blob/40b99c7b57eec10ff8e549a6d6d2b87663804eea/src/iocore/net/NetHandler.cc#L201-L203) - [ ] Remove the handler branches in [`NetHandler.cc` `update_nethandler_config()`](https://github.com/apache/trafficserver/blob/40b99c7b57eec10ff8e549a6d6d2b87663804eea/src/iocore/net/NetHandler.cc#L131-L139) - [ ] Remove the `Dbg()` lines in [`NetHandler.cc` `init_for_process()`](https://github.com/apache/trafficserver/blob/40b99c7b57eec10ff8e549a6d6d2b87663804eea/src/iocore/net/NetHandler.cc#L210-L214) - [ ] Verify `Config` struct index arithmetic ([`updated_member - &global_config[0]`](https://github.com/apache/trafficserver/blob/40b99c7b57eec10ff8e549a6d6d2b87663804eea/src/iocore/net/NetHandler.cc#L154)) still works correctly after removing fields, since [`config_value_affects_per_thread_value`](https://github.com/apache/trafficserver/blob/40b99c7b57eec10ff8e549a6d6d2b87663804eea/include/iocore/net/NetHandler.h#L130) is a bitset indexed by struct field position -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
