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.

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

Online incremental backup #39

Merged
merged 39 commits into from
Apr 26, 2018
Merged

Conversation

RCasatta
Copy link
Member

Upgrade the calendar server with a new endpoint /experimental/backup/[chunk_number].
The endpoint returns a serialized structure containing 1000 timestamp from the calendar server. If [chunk_number] is 0, it returns the commitment from 0 to 999, if 1 from 1000 to 1999 and so on. The commitment number represent the one saved in the calendar journal. By grouping the commitments in chunk of 1000 most of the merkle paths to the block header merkle root are shared and this make the process efficient. Since the client can remember the last chunk asked, the client could keep the backup synced in an incremental fashion avoiding the problems we have in current backup process.

Inside this PR another process entry point is included otsd-backup.py. This launch a process which ask the other calendars for their backups, once he receive the answer, he checks the data contains valid attestation and operations with a local node, if the check passes it writes all the data in a local db. Moreover it contains a stripped down version of the calendar http server with only the /timestamp/ endpoint. Thus a backup server could effectively be queried in place of the calendar server it is backupping,.

RCasatta and others added 30 commits March 3, 2018 09:49
@petertodd
Copy link
Member

  1. Seems to crash if the calendar is empty.
  2. Needs command-line switches for Bitcoin testnet/regtest.

@RCasatta
Copy link
Member Author

first issue should be solved with 416d30a and second issue with 7083f32

otsd-backup.py Outdated
@@ -0,0 +1,101 @@
#!/usr/bin/env python3
# Copyright (C) 2016 The OpenTimestamps developers
Copy link
Member

Choose a reason for hiding this comment

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

s/2016/2018/

Copy link
Member

@petertodd petertodd left a comment

Choose a reason for hiding this comment

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

So far it looks like it works fine in testing!

@@ -0,0 +1,322 @@
#!/usr/bin/env python3
Copy link
Member

Choose a reason for hiding this comment

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

Needs a copyright header.

Also, why is this line here? This file would never be executed.

# TODO next code must be run only once, LOCK or something
backup_map = {}
start = chunk*PAGING
end = start+PAGING
Copy link
Member

Choose a reason for hiding this comment

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

A way to get around the locking issue would be to use a file-based cache, with one file per chunk. Then you could use rename to atomically move each chunk into place after you write it.

Basically do:

with open(chunk_temp_name) as fd:
    <write chunk out>
os.rename(chunk_temp_name, chunk_final_name)

Linux at least rename is atomic.

Copy link
Member

Choose a reason for hiding this comment

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

Oh, and actually, you do use a file-based cache, so just use the rename trick and you'll be fine.

Copy link
Member Author

Choose a reason for hiding this comment

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

should be addressed with 8be5d06

@RCasatta
Copy link
Member Author

Sorry I was super lazy with 8be5d06.
Could you guys check if 78239cd is ok?

with open(cache_file_tmp, 'wb') as fd:
fd.write(bytes)
os.rename(cache_file_tmp, cache_file) # rename is atomic
temp_file = NamedTemporaryFile(delete=False)
Copy link
Member

@petertodd petertodd Apr 26, 2018

Choose a reason for hiding this comment

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

This will fail if /tmp happens to be on a different filesystem than the calendar; use the dir argument to make sure the file is created in the correct directory.

temp_file = NamedTemporaryFile(delete=False)
temp_file.write(bytes)
temp_file.close()
os.rename(temp_file.name, cache_file) # rename is atomic
Copy link
Member

Choose a reason for hiding this comment

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

Have you thought through what happens if this fails?

Since it's atomic, I think it should be enough to simply ignore the error if rename fails due to a file already existing; the file should have the correct content.

Copy link
Member Author

Choose a reason for hiding this comment

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

os.rename does not fail if dest exist, it overwrites, in this case is ok cause the content of the simultaneously created chunk is the same.

Copy link
Member Author

Choose a reason for hiding this comment

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

unless you run Windows, https://docs.python.org/3/library/os.html#os.rename but in that case you are screwed anyway. If could use os.replace if we want to have the same behaviour

@petertodd petertodd merged commit cc5a488 into opentimestamps:master Apr 26, 2018
petertodd added a commit that referenced this pull request Apr 26, 2018
cc5a488 use cache path for temporary file so rename don't fail if /tmp is in another filesystem (Riccardo Casatta)
78239cd proper temp file handling (Riccardo Casatta)
8be5d06 add rename to cache, avoid partial content (Riccardo Casatta)
ffea7b2 add copyright header, remove shebang because not executable and update year (Riccardo Casatta)
e90a940 Do not break the cycle in case http req raise exception, just try later (Riccardo Casatta)
7083f32 select bitcoin network (Riccardo Casatta)
52ba1d5 fix scope visibility in python and add assert (Riccardo Casatta)
416d30a fix access to non existing backup index (Riccardo Casatta)
b546868 use any status code which is not 200 to sleep, nginx  was returning unexpected 502 instead of 404 (Riccardo Casatta)
12dd9b9 sleep for one minute instead of ten between ask to calendars for new chunks (Riccardo Casatta)
e7b98f9 add comments on code (Riccardo Casatta)
c41ce22 create map data iterating in sorted order (Riccardo Casatta)
a0a5c98 support 1 billion commitments for backup cache (Riccardo Casatta)
9e7a76c use binary mode (Riccardo Casatta)
ec8794d create cache path before writing (Riccardo Casatta)
e5ffd61 fix backup path (Riccardo Casatta)
164d9f3 add disk cache (Riccardo Casatta)
8fda365 update backup with new url structure (Riccardo Casatta)
c4247cf set cache for backup files to max (1 year) (Riccardo Casatta)
e70378e fix end header and server path (Riccardo Casatta)
17b0b79 fix wrong import (Riccardo Casatta)
704876a use url path instead of range request (Riccardo Casatta)
1ad0de2 script use only one calendar (Riccardo Casatta)
8fbfecc first backup client script (Riccardo Casatta)
15fa6f6 return inclusive limits (Riccardo Casatta)
1c53843 add range header, limit max requests to 100 (Riccardo Casatta)
0ffdee4 handle commitments up to last (Riccardo Casatta)
6dacb1d fix commitment check in header and function creat_from parameter (Riccardo Casatta)
9d04d96 fix var not None check (Riccardo Casatta)
a9f28c2 add range requests for asking commitments (Riccardo Casatta)
4fb5ddb initial impl of the backup endpoint (Riccardo Casatta)
223a02d serialize and deserialize of kv_map (Riccardo Casatta)
a377ee2 initial test for reading all db and putting it in memory (Riccardo Casatta)

Pull request description:

  Upgrade the calendar server with a new endpoint `/experimental/backup/[chunk_number]`.
  The endpoint returns a serialized structure containing 1000 timestamp from the calendar server. If [chunk_number] is 0, it returns the commitment from 0 to 999, if 1 from 1000 to 1999 and so on. The commitment number represent the one saved in the calendar journal. By grouping the commitments in chunk of 1000 most of the merkle paths to the block header merkle root are shared and this make the process efficient. Since the client can remember the last chunk asked, the client could keep the backup synced in an incremental fashion avoiding the problems we have in current backup process.

  Inside this PR another process entry point is included `otsd-backup.py`. This launch a process which ask the other calendars for their backups, once he receive the answer, he checks the data contains valid attestation and operations with a local node, if the check passes it writes all the data in a local db. Moreover it contains a stripped down version of the calendar http server with only the `/timestamp/` endpoint. Thus a backup server could effectively be queried in place of the calendar server it is backupping,.

Tree-SHA512: 89385d9e33370a85af331cbc4aac50d6bda5c6a175a30e9ff883610f581b6f7accd53d0fb13ebda324eb10285908a4765b1d85bbe02483cf509d1f81a51289cf
@petertodd
Copy link
Member

Looks good to me!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants