chore: Update Raft migrate tool to handle new proposal format #7123

rahulgurnani · 2020-12-11T14:15:31Z

Fixes DGRAPH-2832
This PR updates the raft wal migrate tool. We recently updated raft proposal format for alpha and zero, see #7059 .
The tool now can migrate raftwal from old proposal format to new proposal format.
This change will help to migrate raftwal from RC-2 to RC-4 onwards.

This change is

Earlier we had Key in message Proposal, now it's present outside the Proposal object as first 8 bytes

Earlier we used to store raftwal in badger, so can be deprecated

dgraph/cmd/raft-migrate/run.go

all-seeing-code · 2020-12-11T14:46:20Z

This is to unable migrating from 20.11-rc3 to 20.11.-rc4 without backup restore.

This change is

This is to enable migration or disable migration without backup restore? The comment is unclear without context.

rahulgurnani

Updated the comment, hope it makes sense now.

Reviewable status: 0 of 2 files reviewed, 1 unresolved discussion (waiting on @jarifibrahim, @manishrjain, @NamanJain8, and @vvbalaji-dgraph)

manishrjain

Reviewable status: 0 of 4 files reviewed, 3 unresolved discussions (waiting on @jarifibrahim, @manishrjain, @NamanJain8, @rahulgurnani, and @vvbalaji-dgraph)

dgraph/cmd/raft-migrate/generated.go, line 31 at r3 (raw file):

)

type Proposal struct {

Don't need this file. Protobufs are backwards compatible.

dgraph/cmd/raft-migrate/run.go, line 82 at r3 (raw file):

	newKey := parseAndConvertKey("%02d-%d", oldProposal.Key)
	var newProposal pb.Proposal
	newProposal.Mutations = oldProposal.Mutations

Don't need this either. Protobufs are compatible.

…igrate

NamanJain8

Reviewable status: 0 of 4 files reviewed, 3 unresolved discussions (waiting on @jarifibrahim, @manishrjain, @NamanJain8, and @vvbalaji-dgraph)

dgraph/cmd/raft-migrate/generated.go, line 31 at r3 (raw file):

Previously, manishrjain (Manish R Jain) wrote…

Don't need this file. Protobufs are backwards compatible.

We don't have Key in the new Proposal. Hence, we need the old types(and their unmarshaller) to unmarshal into.

dgraph/cmd/raft-migrate/run.go, line 82 at r3 (raw file):

Previously, manishrjain (Manish R Jain) wrote…

Don't need this either. Protobufs are compatible.

We need all this because of Key.

manishrjain

Reviewable status: 0 of 4 files reviewed, 5 unresolved discussions (waiting on @jarifibrahim, @manishrjain, @NamanJain8, @rahulgurnani, and @vvbalaji-dgraph)

dgraph/cmd/debug/wal.go, line 51 at r4 (raw file):

	var err error
	if isZero {
		var zpr pb.ZeroProposal

The previous code was fine. No need to change this, or add isZero.

dgraph/cmd/debug/wal.go, line 125 at r4 (raw file):

}

func printRaft(db *badger.DB, store *raftmigrate.OldDiskStorage, groupId uint32) {

No need to change.

dgraph/cmd/raft-migrate/generated.go, line 31 at r3 (raw file):

Previously, NamanJain8 (Naman Jain) wrote…

We don't have Key in the new Proposal. Hence, we need the old types(and their unmarshaller) to unmarshal into.

But, that id is reserved still. It should work. Did you try?

ahsanbarkati

Reviewable status: 0 of 4 files reviewed, 7 unresolved discussions (waiting on @jarifibrahim, @manishrjain, @NamanJain8, @rahulgurnani, and @vvbalaji-dgraph)

dgraph/cmd/raft-migrate/run.go, line 162 at r4 (raw file):

	fmt.Printf("Fetching entries from low: %d to high: %d\n", firstIndex, lastIndex)
	// Should we batch this up?
	oldEntries, err := oldWal.Entries(1, lastIndex+1, math.MaxUint64)

should be oldWal.Entries(firstIndex, lastIndex+1, math.MaxUint64)

dgraph/cmd/raft-migrate/run.go, line 196 at r4 (raw file):

	newWal, err := raftwal.InitEncrypted(newDir, encKey)
	x.Check(err)

defer newWal.Close()

NamanJain8

Reviewable status: 0 of 4 files reviewed, 7 unresolved discussions (waiting on @ahsanbarkati, @jarifibrahim, @manishrjain, @rahulgurnani, and @vvbalaji-dgraph)

dgraph/cmd/debug/wal.go, line 51 at r4 (raw file):

Previously, manishrjain (Manish R Jain) wrote…

The previous code was fine. No need to change this, or add isZero.

We noticed that the previous code was not printing out the correct entries for ZeroProposal was able to get unmarshalled into Proposal. Hence, always the first statement was getting executed.

dgraph/cmd/raft-migrate/generated.go, line 31 at r3 (raw file):

Previously, manishrjain (Manish R Jain) wrote…

But, that id is reserved still. It should work. Did you try?

Yes, I tried retrieving Key from the data. But I couldn't find a way unless we write our own unmarhsaller.

dgraph/cmd/raft-migrate/run.go, line 162 at r4 (raw file):

Previously, ahsanbarkati (Ahsan Barkati) wrote…

should be oldWal.Entries(firstIndex, lastIndex+1, math.MaxUint64)

Done.

dgraph/cmd/raft-migrate/run.go, line 196 at r4 (raw file):

Previously, ahsanbarkati (Ahsan Barkati) wrote…

defer newWal.Close()

Done.

manishrjain

Reviewable status: 0 of 4 files reviewed, 7 unresolved discussions (waiting on @ahsanbarkati, @jarifibrahim, @manishrjain, @NamanJain8, @rahulgurnani, and @vvbalaji-dgraph)

dgraph/cmd/raft-migrate/generated.go, line 31 at r3 (raw file):

Previously, NamanJain8 (Naman Jain) wrote…

Yes, I tried retrieving Key from the data. But I couldn't find a way unless we write our own unmarhsaller.

You don't need to retrieve the key from the data. It's not that important. Does something not work without it?

NamanJain8

Reviewable status: 0 of 4 files reviewed, 7 unresolved discussions (waiting on @ahsanbarkati, @jarifibrahim, @manishrjain, @rahulgurnani, and @vvbalaji-dgraph)

dgraph/cmd/raft-migrate/generated.go, line 31 at r3 (raw file):

Previously, manishrjain (Manish R Jain) wrote…

You don't need to retrieve the key from the data. It's not that important. Does something not work without it?

If we don't have a key, then every proposal in raftwal would be considered as invalid. That would mean that we are starting from a fresh state and ignoring the current raftwal https://github.com/dgraph-io/dgraph/blob/11e46217bc09e0e961e167c0a86e427f6f1ff046/dgraph/cmd/zero/raft.go#L336-L338

(cherry picked from commit 941fcda)

dgraph/cmd/raft-migrate/run.go

manishrjain

Do we still have the Badger code? We should remove it.

Reviewable status: 0 of 4 files reviewed, 8 unresolved discussions (waiting on @ahsanbarkati, @manishrjain, @NamanJain8, @rahulgurnani, and @vvbalaji-dgraph)

dgraph/cmd/raft-migrate/generated.go, line 31 at r3 (raw file):

Previously, NamanJain8 (Naman Jain) wrote…

If we don't have a key, then every proposal in raftwal would be considered as invalid. That would mean that we are starting from a fresh state and ignoring the current raftwal https://github.com/dgraph-io/dgraph/blob/11e46217bc09e0e961e167c0a86e427f6f1ff046/dgraph/cmd/zero/raft.go#L336-L338

You can remove that code from Zero.

NamanJain8

Yes, the code in raft-migrate/storage.go should be removed. This code is currently used by dgraph debug tool for debugging old badger wal. @ajeetdsouza is working on it. This code removal can be part of separate PR.

Reviewable status: 0 of 4 files reviewed, 8 unresolved discussions (waiting on @ahsanbarkati, @manishrjain, @NamanJain8, @rahulgurnani, and @vvbalaji-dgraph)

This PR updates the raftmigrate tool. We recently updated raft proposal format for alpha and zero, see #7059 . The tool now can migrate raftwal from old proposal format to new proposal format wherein we are encoding the proposal.Key within the entry.Data. This change will help to migrate raftwal from [RC-2,RC-3] to RC-4 onwards. (cherry picked from commit ab0f3ea)

chore: Update Raft migrate tool to handle new proposal format (#7123) This PR updates the raftmigrate tool. We recently updated raft proposal format for alpha and zero, see #7059 . The tool now can migrate raftwal from old proposal format to new proposal format wherein we are encoding the proposal.Key within the entry.Data. This change will help to migrate raftwal from [RC-2,RC-3] to RC-4 onwards. (cherry picked from commit ab0f3ea) Co-authored-by: Rahul Gurnani <[email protected]>

rahulgurnani added 4 commits December 11, 2020 18:38

Fix dgraph raft wal migrate tool

0c0afc6

Earlier we had Key in message Proposal, now it's present outside the Proposal object as first 8 bytes

Remove older raftwal read code

5b2f9dc

Earlier we used to store raftwal in badger, so can be deprecated

Update alpha proposal entry in slice

6caebb9

Add support for ZeroProposal migration

5fdaf7e

rahulgurnani requested review from manishrjain, jarifibrahim and NamanJain8 December 11, 2020 14:15

rahulgurnani requested a review from vvbalaji-dgraph as a code owner December 11, 2020 14:15

rahulgurnani commented Dec 11, 2020

View reviewed changes

dgraph/cmd/raft-migrate/run.go Outdated Show resolved Hide resolved

rahulgurnani commented Dec 11, 2020

View reviewed changes

ahsanbarkati and others added 2 commits December 12, 2020 19:42

Fix migration of zw

a9ddc18

fix: check entry type before conversion

5b46dec

manishrjain suggested changes Dec 13, 2020

View reviewed changes

NamanJain8 added 6 commits December 14, 2020 13:58

Merge remote-tracking branch 'origin/master' into rahulgurnani/raft-m…

c82a90f

…igrate

handle zero snapshot stored in meta file correctly

0e96441

remove unused flags and update group id

acb933f

clean up

9d48958

fix debug tool for new raft format

571ac44

cleanup

e7221ad

NamanJain8 reviewed Dec 14, 2020

View reviewed changes

manishrjain suggested changes Dec 14, 2020

View reviewed changes

ahsanbarkati suggested changes Dec 14, 2020

View reviewed changes

NamanJain8 reviewed Dec 14, 2020

View reviewed changes

address comments

255e2d5

manishrjain suggested changes Dec 15, 2020

View reviewed changes

NamanJain8 reviewed Dec 15, 2020

View reviewed changes

NamanJain8 and others added 3 commits December 15, 2020 13:19

do err checking on file close

dd7ce33

handle empty entries emmited by raft on leader change

bfead41

Use MembershipState for snapshot

4be83c2

(cherry picked from commit 941fcda)

NamanJain8 added 2 commits December 16, 2020 01:34

ignore the Proposal.Key and simplify the code

da6613d

rename updateProposal

5e26274

jarifibrahim reviewed Dec 16, 2020

View reviewed changes

dgraph/cmd/raft-migrate/run.go Show resolved Hide resolved

store ZeroSnapshot in the zero meta file

fe7f2d0

manishrjain approved these changes Dec 16, 2020

View reviewed changes

NamanJain8 reviewed Dec 16, 2020

View reviewed changes

NamanJain8 merged commit ab0f3ea into master Dec 16, 2020

NamanJain8 deleted the rahulgurnani/raft-migrate branch December 16, 2020 18:48

NamanJain8 restored the rahulgurnani/raft-migrate branch December 24, 2020 08:15

NamanJain8 deleted the rahulgurnani/raft-migrate branch December 24, 2020 11:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: Update Raft migrate tool to handle new proposal format #7123

chore: Update Raft migrate tool to handle new proposal format #7123

rahulgurnani commented Dec 11, 2020 •

edited by NamanJain8

Loading

all-seeing-code commented Dec 11, 2020

rahulgurnani left a comment

manishrjain left a comment

NamanJain8 left a comment

manishrjain left a comment

ahsanbarkati left a comment

NamanJain8 left a comment

manishrjain left a comment

NamanJain8 left a comment

manishrjain left a comment

NamanJain8 left a comment •

edited

Loading

chore: Update Raft migrate tool to handle new proposal format #7123

chore: Update Raft migrate tool to handle new proposal format #7123

Conversation

rahulgurnani commented Dec 11, 2020 • edited by NamanJain8 Loading

all-seeing-code commented Dec 11, 2020

rahulgurnani left a comment

Choose a reason for hiding this comment

manishrjain left a comment

Choose a reason for hiding this comment

NamanJain8 left a comment

Choose a reason for hiding this comment

manishrjain left a comment

Choose a reason for hiding this comment

ahsanbarkati left a comment

Choose a reason for hiding this comment

NamanJain8 left a comment

Choose a reason for hiding this comment

manishrjain left a comment

Choose a reason for hiding this comment

NamanJain8 left a comment

Choose a reason for hiding this comment

manishrjain left a comment

Choose a reason for hiding this comment

NamanJain8 left a comment • edited Loading

Choose a reason for hiding this comment

rahulgurnani commented Dec 11, 2020 •

edited by NamanJain8

Loading

NamanJain8 left a comment •

edited

Loading