Skip to content

Conversation

@antiguru
Copy link
Member

@antiguru antiguru commented Oct 13, 2023

Update Timely and Differential dependencies.

Most of the changes come from TimelyDataflow/differential-dataflow#544.

Motivation

Tips for reviewer

Checklist

  • This PR has adequate test coverage / QA involvement has been duly considered.
  • This PR has an associated up-to-date design doc, is a design doc (template), or is sufficiently small to not require a design.
  • If this PR evolves an existing $T ⇔ Proto$T mapping (possibly in a backwards-incompatible way), then it is tagged with a T-proto label.
  • If this PR will require changes to cloud orchestration or tests, there is a companion cloud PR to account for those changes that is tagged with the release-blocker label (example).
  • This PR includes the following user-facing behavior changes:

@antiguru antiguru changed the title WIP Infer logging IDs by context Update to recent Timely and Differential Dec 19, 2024
@antiguru antiguru marked this pull request as ready for review December 19, 2024 16:21
@antiguru antiguru requested review from a team, aljoscha, danhhz and jkosh44 as code owners December 19, 2024 16:21
@shepherdlybot
Copy link

shepherdlybot bot commented Dec 19, 2024

Risk Score:80 / 100 Bug Hotspots:4 Resilience Coverage:20%

Mitigations

Completing required mitigations increases Resilience Coverage.

  • (Required) Code Review 🔍 Detected
  • (Required) Feature Flag
  • (Required) Integration Test
  • (Required) Observability
  • (Required) QA Review
  • Unit Test
Risk Summary:

The pull request has a high risk score of 80, driven by predictors such as "Sum Bug Reports Of Files" and "Delta of Executable Lines." Historically, PRs with these predictors are 114% more likely to cause a bug than the repository baseline. Although the repo's predicted bug trend is decreasing, the presence of 4 file hotspots suggests elevated caution.

Note: The risk score is not based on semantic analysis but on historical predictors of bug occurrence in the repository. The attributes above were deemed the strongest predictors based on that history. Predictors and the score may change as the PR evolves in code, time, and review activity.

Bug Hotspots:
What's This?

File Percentile
../src/persist_source.rs 96
../src/render.rs 91
../join/linear_join.rs 93
../render/context.rs 96

for monoid in accum.iter() {
datums_local.extend(monoid.finalize().iter());
}
.mz_arrange::<RowBatcher<_, _>, RowBuilder<_, _>, RowSpine<_, Vec<ReductionMonoid>>>(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Around here is where I thought "we have a lot of noise related to these types; would it help to have mz_arrange flavors absorb that complexity?". I don't know if/that it would, but there is an upfront tax to figuring out what to call (perhaps: great, don't add arrangement building operators folks. :D).

Copy link
Contributor

@frankmcsherry frankmcsherry left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I read through everything, and superficially it checks out. I saw one case of a potential bug, though .. once I saw it I realized that there could be other moments like it.

@antiguru antiguru requested a review from a team as a code owner December 19, 2024 19:15
@antiguru antiguru merged commit 6dd6807 into MaterializeInc:main Dec 20, 2024
79 checks passed
@antiguru antiguru deleted the log_identifier branch December 20, 2024 17:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants