Log fold is unstable when doing create and drop table operation and thus affect br restore using cdc log

[yuqi1129]

What problem does this PR solve?

see https://github.com/pingcap/ticdc/issues/1415

What is changed and how it works?

Check List

Tests

• Unit test

Code changes

• Has exported function/method change
• Has exported variable/fields change
• Has interface methods change
• Has persistent data change

Side effects

• Possible performance regression
• Increased code complexity
• Breaking backward compatibility

Related changes

• Need to cherry-pick to the release branch
• Need to update the documentation

Release Note

[3pointer]

I'm afraid this PR will introduce a new issue. I think the proper way to fix it is to choose the proper table id directory with given start-ts and end-ts(not largest table id directory always) in collectRowChangeFiles.

[yuqi1129]

Hi, as far as i can see if a table have several table ids, times spans in each id directory will not intersect. When restore code will choose the correct ids thats contains the restore spans. so logically this will be OK someway, am i right?

Your suggestion is more detailed and nice than this pr, i will follow your advice

3pointer [email protected]于2021年3月9日 周二11:25写道：

I'm afraid this PR will introduce a new issue. I think the proper way to fix it is choose the proper table id directory with given start-ts and end-ts(not largest table id always) in collectRowChangeFiles.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/pingcap/br/pull/739#issuecomment-793321694, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADYQDBBCLAEOPG52ZAVSMIDTCWIKDANCNFSM4X3UXASA .

[yuqi1129]

@3pointer

	nameIdsMap := l.GetNameIDMap()

	log.Debug("nameIdsMap", zap.Any("name and ids:", nameIdsMap))
	for _, ids := range nameIdsMap {
		for _, tID := range ids {
			tableID := tID
			// FIXME update log meta logic here
			dir := fmt.Sprintf("%s%d", tableLogPrefix, tableID)
			opt := &storage.WalkOption{
				SubDir:    dir,
				ListCount: -1,
			}
			err := l.restoreClient.storage.WalkDir(ctx, opt, func(path string, size int64) error {
				fileName := filepath.Base(path)
				shouldRestore, err := l.NeedRestoreRowChange(fileName)
				if err != nil {
					return errors.Trace(err)
				}
				if shouldRestore {
					rowChangeFiles[tableID] = append(rowChangeFiles[tableID], path)
				}
				return nil
			})
			if err != nil {
				return nil, errors.Trace(err)
			}
		}
	}

Hi, The code thats follows l.GetNameIDMap() will traverse cdc storage directory and will choose proper ids thats contains the start time and end time, so i think there is no need to add logic to filter the files, what's your opinion?

[kennytm]

/build

[tisonkun]

@kennytm @3pointer I guess this PR should migrate to pingcap/tidb now? Does it go in a right direction so that we can continue it?