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

Summary Name Collisions - Write in New Format #4706

Closed
arinwt opened this issue Jan 4, 2021 · 5 comments · Fixed by #5280
Closed

Summary Name Collisions - Write in New Format #4706

arinwt opened this issue Jan 4, 2021 · 5 comments · Fixed by #5280
Assignees
Labels
area: runtime Runtime related issues bug Something isn't working
Milestone

Comments

@arinwt
Copy link
Contributor

arinwt commented Jan 4, 2021

This is a follow-up to #3542. After 2 versions are released, we can enable writing in the new format. The previous issue makes it so clients can read/understand the new format.

New tree structure:

[tree] /
    [blob] .metadata **provides snapshot version
    [blob] .chunks
    [tree] .protocol/
    [tree] .blobs/
    [tree] .channels/
        [tree] <dataStoreIds>/
            [blob] .component
            [tree] .channels/
                [tree] <channelIds>/
                    [blob] .attributes
                    [tree] .../
@vladsud
Copy link
Contributor

vladsud commented Jan 5, 2021

Can we do it it Feb, to close #4414 ?

@vladsud vladsud added this to the February 2021 milestone Jan 5, 2021
@ghost ghost added triage and removed triage labels Jan 5, 2021
@arinwt
Copy link
Contributor Author

arinwt commented Jan 6, 2021

Do we want to go ahead and close #4414 since you put in the hotfix resolving the actual bug? This issue is already tracking the long-term fix.

@vladsud
Copy link
Contributor

vladsud commented Jan 6, 2021

I'd want to keep it in some form (as a unique issue not to lose it), to track removal of the hotfix I've put it.

@arinwt
Copy link
Contributor Author

arinwt commented Jan 6, 2021

I see, that makes sense. I think we actually shouldn't remove the hotfix code until +2 versions after this change goes in anyway. If this change goes in version N, then we can expect some N-1, N-2 clients to still write snapshots in the old format, so we probably want to be resilient to those until version N+2 when all clients are writing in the new format.

Even then, new clients opening old snapshots could still encounter the problem, but at least it is unlikely (or at least not guaranteed to encounter it), and if I understood the root issue correctly, it should not be reason to keep the code forever, right?

@vladsud
Copy link
Contributor

vladsud commented Jan 6, 2021

Yes, old files will always be affected. It would be bad if we could not start writing files in new format before GA (end of this quarter), i.e. we should strive to get to a point where shipped product (and files created with shipped product) never have that problem.
For the rest, hard to say. I'd remove the workaround after some time. Note - the issue will shows up only when we do full summary (as we try to rehydrate all DDSs), and I think (need to double-check) that we recover in some form (i.e. it does not result in summaries failing forever from that point, though I might be wrong).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: runtime Runtime related issues bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants