perf(Backup): Improve backup performance #7601

jarifibrahim · 2021-03-17T14:34:02Z

This PR has the following changes

Use thread-local buffer for codecs
Add new ReadTs field to Manifest and use it instead of sinceTs
Use snappy for backups

This change is

- Use thread local buffer for codecs - Disabled Gzip compression (temporarily) - Add new ReadTs field to Manifest and use it instead of sinceTs

…m/backup-improvements

Conflicts: worker/online_restore_ee.go worker/restore.go

NamanJain8

Got few comments. The benefit of introducing ReadTs in the manifest isn't clear to me. Can you please add that in the PR description?

NamanJain8 · 2021-04-05T08:14:26Z

posting/list.go

+	buf.Reset()
+	codec.DecodeToBuffer(buf, ol.Pack)
+	bl.UidBytes = buf.Bytes()
+


Why did we change this heuristic? This change would mean that bl.Uid remains for backward compatibility only.

@manishrjain made this change. We'll have to check with him.

worker/file_handler.go

worker/backup_common.go

jarifibrahim

@NamanJain8 The readTs is added for better readability. The sinceTs used in actually the readTs in case of backups.

jarifibrahim · 2021-04-05T08:56:59Z

posting/list.go

+	buf.Reset()
+	codec.DecodeToBuffer(buf, ol.Pack)
+	bl.UidBytes = buf.Bytes()
+


@manishrjain made this change. We'll have to check with him.

worker/file_handler.go

worker/backup_common.go

NamanJain8 · 2021-04-05T14:33:30Z

Looks good to me.

This reverts commit 2715d88.

This reverts commit 2715d88. We are reverting this change temporarily to bring in other(larger) backup changes. We re-do changes of this commit later.

This commit brings back the changes of PR #7601 which are still valid. - Add new ReadTs feild to Manifest and use it instead of sinceTs

This PR has the following changes - Use thread-local buffer for codecs - Add new ReadTs field to Manifest and use it instead of sinceTs - Use snappy for backups (cherry picked from commit 2715d88)

…#7601) (#7863) This PR has the following changes Use thread-local buffer for codecs (Not applicable to the cherry pick) Add new ReadTs field to Manifest and use it instead of sinceTs Use snappy for backups (Not applicable to the cherry pick) (cherry picked from commit 2715d88) (cherry picked from commit 2715d88) Co-authored-by: Ibrahim Jarif <ibrahim@dgraph.io>

fix(lsbackup): Fix profiler in lsBackup (#7729) Bring back "perf(Backup): Improve backup performance (#7601)" Opt(Backup): Make backups faster (#7680) Fix s3 backup copy (#7669) [BREAKING] Opt(Restore): Optimize Restore's new map-reduce based design (#7666) Perf(restore): Implement map-reduce based restore (#7664) feat(backup): Merge backup refactoring Revert "perf(Backup): Improve backup performance (#7601)"

This commit is a major rewrite of online restore code. It used to use KVLoader in badger. Now it instead uses StreamWriter that is much faster for writes in the case of restore. following commits are cherry-picked (in reverse order): * fix(backup): Free the UidPack after use (#7786) * fix(export-backup): Fix double free in export backup (#7780) (#7783) * fix(lsbackup): Fix profiler in lsBackup (#7729) * Bring back "perf(Backup): Improve backup performance (#7601)" * Opt(Backup): Make backups faster (#7680) * Fix s3 backup copy (#7669) * [BREAKING] Opt(Restore): Optimize Restore's new map-reduce based design (#7666) * Perf(restore): Implement map-reduce based restore (#7664) * feat(backup): Merge backup refactoring * Revert "perf(Backup): Improve backup performance (#7601)"

This commit is a major rewrite of online restore code. It used to use KVLoader in badger. Now it instead uses StreamWriter that is much faster for writes in the case of restore. following commits are cherry-picked (in reverse order): * fix(backup): Free the UidPack after use (#7786) * fix(export-backup): Fix double free in export backup (#7780) (#7783) * fix(lsbackup): Fix profiler in lsBackup (#7729) * Bring back "perf(Backup): Improve backup performance (#7601)" * Opt(Backup): Make backups faster (#7680) * Fix s3 backup copy (#7669) * [BREAKING] Opt(Restore): Optimize Restore's new map-reduce based design (#7666) * Perf(restore): Implement map-reduce based restore (#7664) * feat(backup): Merge backup refactoring * Revert "perf(Backup): Improve backup performance (#7601)" * fix(ee): GetKeys should return an error (#7713) (#7797) * fix(restore): Bump uid and namespace after restore (#7790) (#7800) * fix: fixing graphql schema update when the data is restored + skipping /probe/graphql from audit (#7925) * Don't ban namespace in export_backup

This commit is a major rewrite of backup and online restore code. It used to use KVLoader in badger. Now it instead uses StreamWriter that is much faster for writes. cherry-pick PR #7753 following commits are cherry-picked (in reverse order): * Don't ban namespace in export_backup * fix: fixing graphql schema update when the data is restored + skipping /probe/graphql from audit (#7925) * fix(restore): Bump uid and namespace after restore (#7790) (#7800) * fix(ee): GetKeys should return an error (#7713) (#7797) * fix(backup): Free the UidPack after use (#7786) * fix(export-backup): Fix double free in export backup (#7780) (#7783) * fix(lsbackup): Fix profiler in lsBackup (#7729) * Bring back "perf(Backup): Improve backup performance (#7601)" * Opt(Backup): Make backups faster (#7680) * Fix s3 backup copy (#7669) * [BREAKING] Opt(Restore): Optimize Restore's new map-reduce based design (#7666) * Perf(restore): Implement map-reduce based restore (#7664) * feat(backup): Merge backup refactoring * Revert "perf(Backup): Improve backup performance (#7601)"

This commit is a major rewrite of backup and online restore code. It used to use KVLoader in badger. Now it instead uses StreamWriter that is much faster for writes. cherry-pick PR #7753 following commits are cherry-picked (in reverse order): * reset the kv.StreamId before sending to stream writer (#7833) (#7837) * Don't ban namespace in export_backup * fix: fixing graphql schema update when the data is restored + skipping /probe/graphql from audit (#7925) * fix(restore): Bump uid and namespace after restore (#7790) (#7800) * fix(ee): GetKeys should return an error (#7713) (#7797) * fix(backup): Free the UidPack after use (#7786) * fix(export-backup): Fix double free in export backup (#7780) (#7783) * fix(lsbackup): Fix profiler in lsBackup (#7729) * Bring back "perf(Backup): Improve backup performance (#7601)" * Opt(Backup): Make backups faster (#7680) * Fix s3 backup copy (#7669) * [BREAKING] Opt(Restore): Optimize Restore's new map-reduce based design (#7666) * Perf(restore): Implement map-reduce based restore (#7664) * feat(backup): Merge backup refactoring * Revert "perf(Backup): Improve backup performance (#7601)"

This commit is a major rewrite of backup and online restore code. It used to use KVLoader in badger. Now it instead uses StreamWriter that is much faster for writes. cherry-pick PR #7753 following commits are cherry-picked (in reverse order): * fix(backup): Fix full backup request (#7932) (#7933) * fix: fixing graphql schema update when the data is restored + * fix(restore): return nil if there is error (#7899) skipping /probe/graphql from audit (#7925) * Don't ban namespace in export_backup * reset the kv.StreamId before sending to stream writer (#7833) (#7837) * fix(restore): Bump uid and namespace after restore (#7790) (#7800) * fix(ee): GetKeys should return an error (#7713) (#7797) * fix(backup): Free the UidPack after use (#7786) * fix(export-backup): Fix double free in export backup (#7780) (#7783) * fix(lsbackup): Fix profiler in lsBackup (#7729) * Bring back "perf(Backup): Improve backup performance (#7601)" * Opt(Backup): Make backups faster (#7680) * Fix s3 backup copy (#7669) * [BREAKING] Opt(Restore): Optimize Restore's new map-reduce based design (#7666) * Perf(restore): Implement map-reduce based restore (#7664) * feat(backup): Merge backup refactoring * Revert "perf(Backup): Improve backup performance (#7601)"

This commit is a major rewrite of backup and online restore code. It used to use KVLoader in badger. Now it instead uses StreamWriter that is much faster for writes. cherry-pick PR #7753 following commits are cherry-picked (in reverse order): * opt(restore): Sort the buffer before spinning the writeToDisk goroutine (#7984) (#7996) * fix(backup): Fix full backup request (#7932) (#7933) * fix: fixing graphql schema update when the data is restored + * fix(restore): return nil if there is error (#7899) skipping /probe/graphql from audit (#7925) * Don't ban namespace in export_backup * reset the kv.StreamId before sending to stream writer (#7833) (#7837) * fix(restore): Bump uid and namespace after restore (#7790) (#7800) * fix(ee): GetKeys should return an error (#7713) (#7797) * fix(backup): Free the UidPack after use (#7786) * fix(export-backup): Fix double free in export backup (#7780) (#7783) * fix(lsbackup): Fix profiler in lsBackup (#7729) * Bring back "perf(Backup): Improve backup performance (#7601)" * Opt(Backup): Make backups faster (#7680) * Fix s3 backup copy (#7669) * [BREAKING] Opt(Restore): Optimize Restore's new map-reduce based design (#7666) * Perf(restore): Implement map-reduce based restore (#7664) * feat(backup): Merge backup refactoring * Revert "perf(Backup): Improve backup performance (#7601)"

This commit is a major rewrite of backup and online restore code. It used to use KVLoader in badger. Now it instead uses StreamWriter that is much faster for writes. cherry-pick PR #7753 following commits are cherry-picked (in reverse order): * opt(restore): Sort the buffer before spinning the writeToDisk goroutine (#7984) (#7996) * fix(backup): Fix full backup request (#7932) (#7933) * fix: fixing graphql schema update when the data is restored + skipping /probe/graphql from audit (#7925) * fix(restore): return nil if there is error (#7899) * Don't ban namespace in export_backup * reset the kv.StreamId before sending to stream writer (#7833) (#7837) * fix(restore): Bump uid and namespace after restore (#7790) (#7800) * fix(ee): GetKeys should return an error (#7713) (#7797) * fix(backup): Free the UidPack after use (#7786) * fix(export-backup): Fix double free in export backup (#7780) (#7783) * fix(lsbackup): Fix profiler in lsBackup (#7729) * Bring back "perf(Backup): Improve backup performance (#7601)" * Opt(Backup): Make backups faster (#7680) * Fix s3 backup copy (#7669) * [BREAKING] Opt(Restore): Optimize Restore's new map-reduce based design (#7666) * Perf(restore): Implement map-reduce based restore (#7664) * feat(backup): Merge backup refactoring * Revert "perf(Backup): Improve backup performance (#7601)"

This commit is a major rewrite of backup and online restore code. It used to use KVLoader in badger. Now it instead uses StreamWriter that is much faster for writes. cherry-pick PR #7753 following commits are cherry-picked (in reverse order): * opt(restore): Sort the buffer before spinning the writeToDisk goroutine (#7984) (#7996) * fix(backup): Fix full backup request (#7932) (#7933) * fix: fixing graphql schema update when the data is restored + * fix(restore): return nil if there is error (#7899) skipping /probe/graphql from audit (#7925) * Don't ban namespace in export_backup * reset the kv.StreamId before sending to stream writer (#7833) (#7837) * fix(restore): Bump uid and namespace after restore (#7790) (#7800) * fix(ee): GetKeys should return an error (#7713) (#7797) * fix(backup): Free the UidPack after use (#7786) * fix(export-backup): Fix double free in export backup (#7780) (#7783) * fix(lsbackup): Fix profiler in lsBackup (#7729) * Bring back "perf(Backup): Improve backup performance (#7601)" * Opt(Backup): Make backups faster (#7680) * Fix s3 backup copy (#7669) * [BREAKING] Opt(Restore): Optimize Restore's new map-reduce based design (#7666) * Perf(restore): Implement map-reduce based restore (#7664) * feat(backup): Merge backup refactoring * Revert "perf(Backup): Improve backup performance (#7601)"

This commit is a major rewrite of backup and online restore code. It used to use KVLoader in badger. Now it instead uses StreamWriter that is much faster for writes. cherry-pick PR #7753 following commits are cherry-picked (in reverse order): * opt(restore): Sort the buffer before spinning the writeToDisk goroutine (#7984) (#7996) * fix(backup): Fix full backup request (#7932) (#7933) * fix: fixing graphql schema update when the data is restored + skipping /probe/graphql from audit (#7925) * fix(restore): return nil if there is error (#7899) * Don't ban namespace in export_backup * reset the kv.StreamId before sending to stream writer (#7833) (#7837) * fix(restore): Bump uid and namespace after restore (#7790) (#7800) * fix(ee): GetKeys should return an error (#7713) (#7797) * fix(backup): Free the UidPack after use (#7786) * fix(export-backup): Fix double free in export backup (#7780) (#7783) * fix(lsbackup): Fix profiler in lsBackup (#7729) * Bring back "perf(Backup): Improve backup performance (#7601)" * Opt(Backup): Make backups faster (#7680) * Fix s3 backup copy (#7669) * [BREAKING] Opt(Restore): Optimize Restore's new map-reduce based design (#7666) * Perf(restore): Implement map-reduce based restore (#7664) * feat(backup): Merge backup refactoring * Revert "perf(Backup): Improve backup performance (#7601)"

NamanJain8 and others added 6 commits March 15, 2021 22:17

fix crash due to telemetry

8f710e3

perf(backup): Improve backup performance

2d064c7

- Use thread local buffer for codecs - Disabled Gzip compression (temporarily) - Add new ReadTs field to Manifest and use it instead of sinceTs

Merge remote-tracking branch 'origin/naman/fix-telemetry' into ibrahi…

734b50c

…m/backup-improvements

Small changes to optimize memory usage

1e993fe

Improve write performance with Snappy

349a00a

fix restore code to use compression

d6d485d

jarifibrahim requested review from manishrjain, martinmr, pawanrawal and vvbalaji-dgraph as code owners March 17, 2021 14:34

Ibrahim Jarif added 7 commits March 17, 2021 20:47

Merge branch 'release/v21.03' into ibrahim/backup-improvements

cfc1747

fix failing test

9f2db01

Merge branch 'release/v21.03' into ibrahim/backup-improvements

381f4aa

Conflicts: worker/online_restore_ee.go worker/restore.go

fix more tests

0f55f84

Fixup

720c61b

fix codec tests

17c048b

Merge branch 'release/v21.03' into ibrahim/backup-improvements

f789807

NamanJain8 reviewed Apr 5, 2021

View reviewed changes

add review comments

4055833

jarifibrahim commented Apr 5, 2021

View reviewed changes

NamanJain8 approved these changes Apr 5, 2021

View reviewed changes

Merge branch 'release/v21.03' into ibrahim/backup-improvements

bc8b7b4

jarifibrahim merged commit 2715d88 into release/v21.03 Apr 6, 2021

jarifibrahim deleted the ibrahim/backup-improvements branch April 6, 2021 07:20

ahsanbarkati added a commit that referenced this pull request Apr 22, 2021

Revert "perf(Backup): Improve backup performance (#7601)"

760a827

This reverts commit 2715d88.

ahsanbarkati added a commit that referenced this pull request Apr 23, 2021

Revert "perf(Backup): Improve backup performance (#7601)"

942c4a7

This reverts commit 2715d88. We are reverting this change temporarily to bring in other(larger) backup changes. We re-do changes of this commit later.

ahsanbarkati added a commit that referenced this pull request Apr 23, 2021

Bring back "perf(Backup): Improve backup performance (#7601)"

f89d3b4

This commit brings back the changes of PR #7601 which are still valid. - Add new ReadTs feild to Manifest and use it instead of sinceTs

mangalaman93 mentioned this pull request Dec 13, 2022

fix(backup): use StreamWriter instead of KVLoader during backup restore #8510

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

perf(Backup): Improve backup performance #7601

perf(Backup): Improve backup performance #7601

jarifibrahim commented Mar 17, 2021 •

edited

Loading

NamanJain8 left a comment

NamanJain8 Apr 5, 2021

jarifibrahim Apr 5, 2021 •

edited

Loading

jarifibrahim left a comment

jarifibrahim Apr 5, 2021 •

edited

Loading

NamanJain8 commented Apr 5, 2021

perf(Backup): Improve backup performance #7601

perf(Backup): Improve backup performance #7601

Conversation

jarifibrahim commented Mar 17, 2021 • edited Loading

NamanJain8 left a comment

Choose a reason for hiding this comment

NamanJain8 Apr 5, 2021

Choose a reason for hiding this comment

jarifibrahim Apr 5, 2021 • edited Loading

Choose a reason for hiding this comment

jarifibrahim left a comment

Choose a reason for hiding this comment

jarifibrahim Apr 5, 2021 • edited Loading

Choose a reason for hiding this comment

NamanJain8 commented Apr 5, 2021

jarifibrahim commented Mar 17, 2021 •

edited

Loading

jarifibrahim Apr 5, 2021 •

edited

Loading

jarifibrahim Apr 5, 2021 •

edited

Loading