DM-55232: fix zstd failure in aggregate-graph due to huge logs#565
DM-55232: fix zstd failure in aggregate-graph due to huge logs#565TallJimbo wants to merge 2 commits into
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #565 +/- ##
==========================================
- Coverage 88.79% 88.67% -0.13%
==========================================
Files 160 160
Lines 22120 22150 +30
Branches 2625 2627 +2
==========================================
Hits 19641 19641
- Misses 1843 1866 +23
- Partials 636 643 +7 ☔ View full report in Codecov by Harness. |
TallJimbo
force-pushed
the
tickets/DM-55232
branch
from
07e4ee7 to
b017815
Compare
MichelleGower
left a comment
MichelleGower
left a comment
There was a problem hiding this comment.
A couple comments. Merge approved.
| break | ||
| if training_sample_size >= self.comms.config.zstd_dict_input_max_bytes: | ||
| self.comms.log.warning( | ||
| "Truncating compression dict training sample after %d predicted quanta.", |
There was a problem hiding this comment.
I don't know if it would be quicker to understand if instead of saying "Truncating" saying something like "Reached compression dict training sample limit after..."
There was a problem hiding this comment.
Also, do you want a similar warning with the above break for counts?
| pred_quanta_sum = sum(len(block) for block in training_inputs[:n_pred_quanta]) | ||
| prov_quanta_sum = sum(len(block) for block in training_inputs[n_pred_quanta::3]) | ||
| metadata_sum = sum(len(block) for block in training_inputs[n_pred_quanta + 1 :: 3]) | ||
| logs_sum = sum(len(block) for block in training_inputs[n_pred_quanta + 2 :: 3]) |
There was a problem hiding this comment.
I possibly would set the step (3) in a variable to make sure future changes get them all. But the lines are all here together so...
If it is easy to write a unit test to catch if we add/remove something to/from the training_inputs but forget to change the step, that could avoid trying to track down a bug in the exception handling code.
2 participants
Checklist
package-docs builddoc/changes