Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Idea] Replace monolithic Sync diff with per-record diffs #623

Open
glennmatthews opened this issue Dec 10, 2024 · 5 comments
Open

[Idea] Replace monolithic Sync diff with per-record diffs #623

glennmatthews opened this issue Dec 10, 2024 · 5 comments
Labels
status: action required This issue requires additional information to be actionable status: gathering feedback Further discussion is needed to determine this issue's scope and/or implementation status: internal review Internal discussion is required to move forward with issue type: major feature

Comments

@glennmatthews
Copy link
Contributor

Environment

  • Nautobot version: all
  • nautobot-ssot version: all

Proposed Functionality

As noted in #144, #53, #140, etc., the current approach of storing the entire diff set for a given sync run in a single JSONField in a single Sync record doesn't scale well at all. To be able to usefully and performantly represent syncs involving 10k, 100k, or more changed objects, we should consider a different database representation for diffs.

The most "obvious" approach would be a single database record per changed object, with the UI taking responsibility for asynchronously fetching all relevant records associated with a given sync in order to provide a unified view of the synced changes as a whole.

I would say "just use core's ObjectChange model for this", but:

  1. that model stores a snapshot rather than diffs (at least for now)
  2. we need the ability to store "speculative" changes (i.e. dry run) in addition to "as-applied" changes.

So given that, we probably need a dedicated ObjectDiff model specifically in SSOT.

Use Case

Scalability and performance.

@jdrew82
Copy link
Contributor

jdrew82 commented Dec 10, 2024

To add to this, I think it'd also be a good solution to the "caching" use case where we want to use previous data from a dryrun and execute on it without having to reprocess everything.

@jdrew82 jdrew82 added status: gathering feedback Further discussion is needed to determine this issue's scope and/or implementation status: action required This issue requires additional information to be actionable status: internal review Internal discussion is required to move forward with issue type: major feature labels Dec 10, 2024
@Kircheneer
Copy link
Contributor

Related: #127

@Kircheneer
Copy link
Contributor

Any chance we can maybe use a foreign key to the object change model so we don't have to store everything twice? Perhaps the diff can be ad-hoc calculated from the object change records?

Although I guess this doesn't help with the speculative use case.

@glennmatthews
Copy link
Contributor Author

Using ObjectChange also wouldn't help with outbound syncs (Nautobot-to-other-system), but it's worth consideration anyways.

@jdrew82
Copy link
Contributor

jdrew82 commented Dec 16, 2024

The non-Nautobot SoR use cases would be my primary reason for not wanting to have that direct link. Also, I don't think it's that easy to grep what change happened from those records. The current diff that's shown in SyncLogs is good but might be better to show the full before/after of the loaded object and potentially a link back to the previous change for that object so you could do a "git diff history" type tracking for objects?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: action required This issue requires additional information to be actionable status: gathering feedback Further discussion is needed to determine this issue's scope and/or implementation status: internal review Internal discussion is required to move forward with issue type: major feature
Projects
None yet
Development

No branches or pull requests

3 participants