Fix local commit resubmission patchset tracking#211
Open
smcdef wants to merge 3 commits into
Open
Conversation
Local commit ingestion uses the commit SHA as patches.message_id. That SHA is not globally unique as a patch identity: the same commit can be submitted as part of a large range, and later submitted again as a singleton or as part of an overlapping range. The old patches table made message_id globally unique. When the later submission inserted the same commit SHA, create_patch() treated it as an upsert of the existing patch row and changed its patchset_id to the new patchset. The earlier range then silently lost that patch row. Make patch identity unique per patchset instead. Migrate old databases by rebuilding the patches table without the global message_id unique constraint, and add a composite unique index on (patchset_id, message_id). Update create_patch() to upsert only within the target patchset. Also drop duplicate patch rows before merging patchsets so the composite unique constraint cannot leave rows attached to a deleted patchset. Signed-off-by: Muchun Song <songmuchun@bytedance.com>
The API creates a Fetching patchset before handing a local commit request to the fetch worker. If the user submits a short SHA, that placeholder is keyed by the short SHA plus the synthetic @sashiko.local suffix. The fetch worker resolves the commit to a full SHA before ingestion. Singleton ingestion then used the full SHA as the cover id, so it missed the short-SHA placeholder and created a second real patchset. The original placeholder stayed visible as Fetching forever. Use the submitted article id to match git-fetch/api-submit placeholders, then canonicalize singleton patchsets back to the resolved full SHA after the real message has been stored. Also migrate old databases by deleting empty stale local Fetching placeholders when a matching singleton patchset already exists, preserving skip/only filters on the real patchset. Signed-off-by: Muchun Song <songmuchun@bytedance.com>
Submitting the same local commit SHA after a completed review could make clients look up the old patchset again. The submit API returned the SHA as the accepted id, and message-id lookup did not choose the newest matching patchset, so the UI could show the previous review while the new run was still in progress. Return the placeholder patchset id from remote submissions, prefer the newest patchset for message-id detail lookups, and create a new fetching patchset when a completed local commit is submitted again. Also prefer active placeholders when ingest attaches fetched patches. Route patchset list entries by their numeric patchset id instead of the message id, so repeated submissions of the same local SHA open distinct review pages. Signed-off-by: Muchun Song <songmuchun@bytedance.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fix local commit submission tracking when the same commit SHA is submitted
multiple times, either as a singleton or as part of overlapping ranges.
Previously,
patches.message_idwas globally unique. Local commit ingestionuses the commit SHA as
message_id, so ingesting the same commit in anotherpatchset could move the existing patch row to the new patchset. This made the
older patchset silently lose that patch.
This PR changes patch identity to be unique per patchset, improves placeholder
reuse for short SHA submissions, and makes repeated completed local submissions
open distinct review runs.
Changes
patches.message_idunique per patchset via(patchset_id, message_id).create_patch()so duplicate local commit SHAs are only upsertedwithin the same patchset.
patches:
reviews.patch_idemail_outbox.patch_idpatches_subsystems.patch_idto a full commit hash.
patchsets.
the exact review run.
Testing
Added regression coverage for:
message_iduniquenessRan full test suite:
All tests passed.