Skip to content

merge recovery: optimization: use copy-iterator & server-send for small/medium keys - #678

Merged
bioothod merged 6 commits into
reverbrain:masterfrom
abudnik:recovery_merge
Dec 10, 2015
Merged

merge recovery: optimization: use copy-iterator & server-send for small/medium keys#678
bioothod merged 6 commits into
reverbrain:masterfrom
abudnik:recovery_merge

Conversation

@abudnik

@abudnik abudnik commented Dec 7, 2015

Copy link
Copy Markdown
Member

No description provided.

Comment thread recovery/elliptics_recovery/iterator.py Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What are the destination groups?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

CopyIterator is used in merge recovery only, that's why exactly one group specified here.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

CopyIterator name implies the opposite, please add documentation describing how it is used and better change the name

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Ok

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@bioothod

bioothod commented Dec 7, 2015

Copy link
Copy Markdown
Member

This pull request contains fair amount of code from #676, presumably it depends on #676, but the former is not yet ready, is that correct?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please add description of how does this class work, what is the logic behind it and so on, as much information as possible

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

done

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This class doesn't update any statistics, so it is impossible to track recovery progress.

@abudnik

abudnik commented Dec 8, 2015

Copy link
Copy Markdown
Member Author

This pull request contains fair amount of code from #676, presumably it depends on #676, but the former is not yet ready, is that correct?

Yes. Also, #668 depends on #676.

bioothod added a commit that referenced this pull request Dec 10, 2015
merge recovery: optimization: use copy-iterator & server-send for small/medium keys
@bioothod
bioothod merged commit 32d47d7 into reverbrain:master Dec 10, 2015

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It should also update statistics in own way, because now it will not show any recovering progress and all iterator's statistics will be about keys which weren't recovered by copy-iterator.

@abudnik abudnik mentioned this pull request Dec 15, 2015

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

for key_id, key_responses in responses.iteritems() to get rid of val[1] and val[0]

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please, take a look at #679 where this PR was complemented.

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.

4 participants