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.

Sign up for GitHub

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

Jump to bottom

VStreamer: Schema versioning #6164

Merged
merged 17 commits into from
May 31, 2020
Merged

VStreamer: Schema versioning #6164

merged 17 commits into from
May 31, 2020

Conversation

rohit-nayak-ps
Copy link
Contributor

No description provided.

@rohit-nayak-ps rohit-nayak-ps requested a review from sougou as a code owner May 7, 2020 20:23
sougou
sougou reviewed May 14, 2020
View reviewed changes
Copy link
Contributor

@sougou sougou left a comment

Choose a reason for hiding this comment

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

Some feedback life-cycle management, which I forgot to ramp you up on.
Also, I think we can simplify historian's code as mentioned in the comment.

go/vt/vttablet/endtoend/vstreamer_test.go Show resolved Hide resolved
go/vt/vttablet/tabletserver/tabletserver.go Outdated Show resolved Hide resolved
go/vt/vttablet/tabletserver/tabletserver.go Outdated Show resolved Hide resolved
go/vt/vttablet/tabletserver/tabletserver.go Show resolved Hide resolved
go/vt/vttablet/tabletserver/schema/historian.go Outdated Show resolved Hide resolved
sougou
sougou reviewed May 25, 2020
View reviewed changes
Copy link
Contributor

@sougou sougou left a comment

Choose a reason for hiding this comment

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

Nice work. I added some nit-picks.

go/vt/vttablet/tabletserver/schema/historian.go Outdated Show resolved Hide resolved
go/vt/vttablet/tabletserver/schema/historian.go Outdated Show resolved Hide resolved
go/vt/vttablet/tabletserver/schema/historian.go Outdated Show resolved Hide resolved
go/vt/vttablet/tabletserver/schema/historian.go Outdated Show resolved Hide resolved
go/vt/vttablet/tabletserver/schema/historian.go
tables[t.Name] = t
}
trackedSchema := &TrackedSchema{
schema: tables,
Copy link
Contributor

Choose a reason for hiding this comment

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

Since sch doesn't escape out of this function, it's better to directly assign sch.Tables. No need to copy.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sch.Tables is a list, converting to a map.

go/vt/vttablet/tabletserver/schema/historian.go Outdated Show resolved Hide resolved
go/vt/vttablet/tabletserver/schema/historian.go Outdated Show resolved Hide resolved
go/vt/vttablet/tabletserver/tabletenv/config.go Show resolved Hide resolved
go/vt/vttablet/tabletserver/tabletserver.go Show resolved Hide resolved
systay and others added 17 commits May 31, 2020 00:16
@systay @rohit-nayak-ps
Update replication watcher to look for DDLs
512f6e5 
Signed-off-by: Andres Taylor <[email protected]>
@systay @rohit-nayak-ps
wip
12c3da6 
Signed-off-by: Rohit Nayak<[email protected]>

Signed-off-by: Rohit Nayak <[email protected]>
@rohit-nayak-ps
WIP
7e1706d 
Signed-off-by: Rohit Nayak <[email protected]>
@rohit-nayak-ps
Basic functionality implemented. More tests needed
7b54e2a 
Signed-off-by: Rohit Nayak <[email protected]>
@rohit-nayak-ps
Working version with e2e for 'current' stream
9138ede 
Signed-off-by: Rohit Nayak <[email protected]>
@rohit-nayak-ps
Working version with e2e for 'current' stream
f3ac8bd 
Signed-off-by: Rohit Nayak <[email protected]>
@rohit-nayak-ps
e2e test for past events with and without tracking
a22b5ea 
Signed-off-by: Rohit Nayak <[email protected]>
@rohit-nayak-ps
Add comments to explain generated events
98e80a8 
Signed-off-by: Rohit Nayak <[email protected]>
@rohit-nayak-ps
Working unit tests for schema_versioning
7191d1a 
Signed-off-by: Rohit Nayak <[email protected]>
@rohit-nayak-ps
Rename protos
8104d77 
Signed-off-by: Rohit Nayak <[email protected]>
@rohit-nayak-ps
Fix incorrect commit
80be5de 
Signed-off-by: Rohit Nayak <[email protected]>
@rohit-nayak-ps
Fix race
e877676 
Signed-off-by: Rohit Nayak <[email protected]>
@rohit-nayak-ps
Code cleanup
88b9982 
Signed-off-by: Rohit Nayak <[email protected]>
@rohit-nayak-ps
Incorporate review comments
cdd3f60 
Signed-off-by: Rohit Nayak <[email protected]>
@rohit-nayak-ps
Fixed e2e test
23460e3 
Signed-off-by: Rohit Nayak <[email protected]>
@rohit-nayak-ps
Addressed review comments
84fb821 
Signed-off-by: Rohit Nayak <[email protected]>
@rohit-nayak-ps
Historian now subscribes to schema engine. Added ReloadAt in SE/VS to…
81512fa 
… optimize schema loads. Related tests. Fixed TestMessage race

Signed-off-by: Rohit Nayak <[email protected]>
@sougou sougou merged commit b28fb8f into vitessio:master May 31, 2020
@rohit-nayak-ps rohit-nayak-ps linked an issue Jun 10, 2020 that may be closed by this pull request
RFC: Tracking schema changes in vstreams #6300
Closed
@deepthi deepthi added this to the v7.0 milestone Jul 17, 2020
@rohit-nayak-ps rohit-nayak-ps mentioned this pull request Jul 25, 2020
Closed
@ameetkotian ameetkotian mentioned this pull request Aug 20, 2020
Merged
@indera-shsp
Copy link
Contributor

indera-shsp commented Jul 1, 2021 •
edited
Loading

This implements rfc - #6300

From a Slack discussion -- Rohit mentioned

If you stream from the primary the vstream will handle DDLs correctly. 
You should run the tracker on the primary. If you are streaming realtime then you don't need the tracker 
(the stream will pause while the latest schema is loaded), 
but if streams are run for older events then the tracker will provide the correct schema for the older events.

You should run the tracker on the primary

@rohit-nayak-ps What does primary mean here? My interpretation is that we should enable these two flags

-watch_replication_stream and -track_schema_versions

on all replica tablets since any of them can become primary.

@indera-shsp indera-shsp mentioned this pull request Jul 1, 2021
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sougou sougou sougou approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v7.0
Development

Successfully merging this pull request may close these issues.

RFC: Tracking schema changes in vstreams
5 participants
@rohit-nayak-ps @indera-shsp @sougou @deepthi @systay