Limit logging of soft errors #245
Conversation
- Revised earlier changes to limit logging of ALREADY_EXISTS errors. Defined ALREADY_EXISTS and NOT_FOUND as soft errors, and implemented IsSoftError() and IsSoftTdiError() predicates to check whether a status code is a soft error. Signed-off-by: Derek Foster <[email protected]>
Signed-off-by: Derek Foster <[email protected]>
Signed-off-by: Derek Foster <[email protected]>
- Renamed filtering macro to RETURN_IF_ERROR_WITH_FILTERING(). - Add filtering to calls to GetTableEntry(). Signed-off-by: Derek Foster <[email protected]>
Signed-off-by: Derek Foster <[email protected]>
|
|
||
| // Checks whether a TDI status code is a soft error. | ||
| static inline bool IsSoftTdiError(tdi_status_t err) { | ||
| return (err == TDI_ALREADY_EXISTS) || (err == TDI_OBJECT_NOT_FOUND) || |
There was a problem hiding this comment.
What is the definition of a soft error in this implementation? Why is "object/table not found" considered a soft error?
There was a problem hiding this comment.
-
When I reviewed the updated HSD-ES ticket, I discovered that it contained log messages like this:
E20231201 00:28:53.970028 5140 tdi_table_manager.cc:624] Return Error: tdi_sde_interface_->GetTableEntry(device_, session, table_id, table_key.get(), table_data.get()) failed with StratumErrorSpace::ERR_ENTRY_NOT_FOUND: 'table->entryGet(*real_session->tdi_session_, *dev_tgt, flags, *real_table_key->table_key_, real_table_data->table_data_.get())' failed with error message: Object not found. E20231201 00:28:53.970144 5140 es2k_node.cc:278] StratumErrorSpace::ERR_AT_LEAST_ONE_OPER_FAILED: One or more read operations failed. E20231201 00:28:53.970456 5140 p4_service.cc:353] Failed to read forwarding entries from node 1: One or more read operations failed. -
The commentary showed that they were also bothered about these issues (i.e., my previous commit had addressed only half their problem).
-
ALREADY_EXISTS and NOT_FOUND are two sides of the same coin. The conditions are easily recoverable, which is what makes them "soft" errors.
-
The customer is complaining about the fact that the intermediate code is logging, not just ALREADY_EXISTING, but ALREADY_EXISTING and NOT_FOUND. They are therefore complaining about our logging soft errors.
-
When TDI status codes are converted to Stratum status codes, TDI_TABLE_NOT_FOUND and TDI_OBJECT_NOT_FOUND are both mapped to ERR_ENTRY_NOT_FOUND. I therefore treat them as identical.
-
The idea is to reduce the amount of special-purpose code by treating all errors with this classification identically.
There was a problem hiding this comment.
Point taken. I've added some additional documentation to the header file.
Signed-off-by: Derek Foster <[email protected]>
Revised and expanded on the changes made in PR #240.
IsSoftError()predicates to check whether a status code is a soft error.