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

Add backup count option #4

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
Open

Add backup count option #4

wants to merge 11 commits into from

Conversation

BG4444
Copy link

@BG4444 BG4444 commented Sep 8, 2018

Automatically maintain count of backups and rebase first backup in chain.

@@ -130,7 +131,7 @@ def do_fallocate(filename: str, size: int):
os.fsync(xxx.fileno())
log.debug('Fsyncing complete.')

os.chmod(tmp_filename, 0o400)
#os.chmod(tmp_filename, 0o400)
Copy link
Owner

Choose a reason for hiding this comment

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

Но зачем? все генерируемые файлы должны быть ридонли. это защита от случайных правок (не по друтом которые)

Copy link
Author

Choose a reason for hiding this comment

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

Он же делает ребейс. Можно конечно покрасивше сделать - скидывать атрибут непосредственно перед ребейсом и после ребейса обратно ставить.

Copy link
Owner

Choose a reason for hiding this comment

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

может лутьше отдельную тулзу написать ? (т.е. не городить в этом питоне) -- бекапилка-бекапилкой, а удалялка тершака - отдельно. запускать их можно независимо в теории.

Copy link
Author

Choose a reason for hiding this comment

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

Ну, тут сложна-сложна. По идее, это один процесс - хлопнуть один лишний бекап - добавить один новый бекап. Одна строка в кроне. Культурненько.

@@ -147,20 +148,18 @@ def do_fallocate(filename: str, size: int):
raise


def get_latest_backup(xxx: str, image_name: str) -> int:
def get_latest_backup(xxx: str, image_name: str)->dict:
Copy link
Owner

@socketpair socketpair Sep 17, 2018

Choose a reason for hiding this comment

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

  1. отинденти код как минимум (Pycharm).
  2. -> Dict[int, str]

Copy link
Author

Choose a reason for hiding this comment

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

я не уметь, питона в первый раз в руки взял

Copy link
Owner

Choose a reason for hiding this comment

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

поэтому я и подсказываю

@@ -339,6 +366,9 @@ def main():
if not (1 <= options.parallel <= 100):
raise ValueError('Wrong parallel count.')

if not (options.bk_count >= 4):
raise ValueError('Wrong backups count. Minimum is 4.')
Copy link
Owner

Choose a reason for hiding this comment

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

С чего это ради.

Copy link
Author

Choose a reason for hiding this comment

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

Ну, если меньше четырех, то ребейсить некуда. Пушо получается, что самый первый бекап в цепочке он самый большой, поэтому на него ребейсить будет долго (наверное)

metavar='BK_COUNT',
type=int,
help='Count of backups to store.',
default=sys.maxsize
Copy link
Owner

Choose a reason for hiding this comment

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

упоролся чтоли. по дефолту 1 же логичнее :)

Copy link
Author

Choose a reason for hiding this comment

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

Ну, тогда он будет держать ровно 1 бекап, остальные грохать

Copy link
Owner

Choose a reason for hiding this comment

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

бля, я вначале думал -- сколько снапов в цефе будет оставаться.

'--bk_count',
metavar='BK_COUNT',
type=int,
help='Count of backups to store.',
Copy link
Owner

@socketpair socketpair Sep 17, 2018

Choose a reason for hiding this comment

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

to store on Ceph (!) нет ?

Copy link
Author

Choose a reason for hiding this comment

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

нит, то сторе он винт!

os.path.join(xxx, itms[srt[removingCount+2]])
];
log.info('Rebasing image %s.', args)
subprocess.check_call(args)
Copy link
Owner

Choose a reason for hiding this comment

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

если упадёт, то надо рековериться. ну и защита от параллельного бекапенья же в этом время

Copy link
Author

Choose a reason for hiding this comment

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

Вот тут не знаю. Если ребейз упал, по идее, всей цепочке бекапов звездык

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.

2 participants