syncoid: Compare existing bookmarks #626
Conversation
0xFelix
force-pushed
the
ignore-failed-create-bookmark
branch
2 times, most recently
from
a478599 to
7e76de6
Compare
|
@jimsalterjrs @phreaker0 Ping |
0xFelix
force-pushed
the
ignore-failed-create-bookmark
branch
from
7e76de6 to
2979a7e
Compare
0xFelix
force-pushed
the
ignore-failed-create-bookmark
branch
from
2979a7e to
fd449dd
Compare
|
I think a better way to handle this would be to check the bookmark on failure if it already exists and points to the correct snapshot and if not still fail. So there wouldn't be the need for an extra argument and one can be sure that the bookmark is correct. |
When creating bookmarks compare the GUID of existing bookmarks before failing the creation of a duplicate bookmark. Signed-off-by: 0xFelix <[email protected]>
0xFelix
force-pushed
the
ignore-failed-create-bookmark
branch
from
fd449dd to
dd244de
Compare
0xFelix
changed the title
|
@phreaker0 Makes sense! Updated the PR to implement that. |
|
Also ping @jimsalterjrs |
| if (defined $args{'create-bookmark'}) { | ||
| my $ret = createbookmark($sourcehost, $sourcefs, $newsyncsnap, $newsyncsnap); | ||
| my %existingbookmarks = getbookmarks($sourcehost,$sourcefs,$sourceisroot); | ||
| my $guid = $snaps{'source'}{$newsyncsnap}{'guid'}; | ||
| my $ret = createbookmark($sourcehost, $sourcefs, $newsyncsnap, $newsyncsnap, $guid, %existingbookmarks); | ||
| $ret == 0 or do { |
There was a problem hiding this comment.
this code should stay as it where before, because doing it this way will degrade performance because each time all the bookmarks needs to be listed.
It's better to handle that in the fallback case (if the bookmark creation fails), there the bookmark listing and the compare if this bookmark already exists should be made.
| my ($sourcehost, $sourcefs, $snapname, $bookmark, $guid, %existingbookmarks) = @_; | ||
|
|
||
| if (defined $existingbookmarks{$guid} && $existingbookmarks{$guid}{'name'} eq $bookmark) { | ||
| writelog('INFO', "bookmark already exists, skipping creation"); | ||
| return 0; | ||
| } |
There was a problem hiding this comment.
this should be move to the fallback case if the bookmark creation failed
| @@ -0,0 +1,93 @@ | |||
| #!/bin/bash | |||
There was a problem hiding this comment.
because of #904 this needs to be renamed with a 0 prefix.
When creating bookmarks compare the GUID of existing bookmarks before
failing the creation of a duplicate bookmark.