Skip to content

Initial shard aware driver - #1

Open
fruch wants to merge 3 commits into
masterfrom
scylla-3.21.0-shard-aware
Open

Initial shard aware driver#1
fruch wants to merge 3 commits into
masterfrom
scylla-3.21.0-shard-aware

Conversation

@fruch

@fruchfruch commented Feb 21, 2020

Copy link
Copy Markdown
Owner

Temporary PR so we could have comments

vladzcloudius
vladzcloudius suggested changes Feb 27, 2020
View reviewed changes
Comment threadcassandra/pool.py Outdated
Comment threadcassandra/pool.py Outdated
Comment threadcassandra/pool.py Outdated
Comment threadcassandra/pool.py
vladzcloudius
vladzcloudius suggested changes Mar 2, 2020
View reviewed changes
Comment threadcassandra/connection.py
_sum = (mul1 >> 32) + mul2
output = (_sum >> 32)
print(output)
output = _sum >> 32

@vladzcloudiusvladzcloudiusMar 2, 2020

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Hmmm... What is this about anyway? ;)

Comment threadcassandra/pool.py
Comment threadcassandra/pool.py
vladzcloudius
vladzcloudius suggested changes Mar 2, 2020
View reviewed changes
Comment threadtests/integration/standard/test_shard_aware.py Outdated
Comment threadtests/integration/standard/test_shard_aware.py Outdated
Comment threadtests/unit/test_shard_aware.py
from cassandra.metadata import Murmur3Token

class TestShardAware(unittest.TestCase):
def test_parsing_and_calculating_shard_id(self):

@vladzcloudiusvladzcloudiusMar 2, 2020

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The same here: some human readable description please. See examples in (at least mine) dtests.
For instance in hintedhandoff_additional_tests.py.

@fruch
fruchforce-pushed the scylla-3.21.0-shard-aware branch 4 times, most recently from c619e5b to 16a903dCompareMarch 11, 2020 16:27
vladzcloudius
vladzcloudius suggested changes Mar 11, 2020
View reviewed changes

@vladzcloudiusvladzcloudius left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Some more detailed description as to why this patch is needed would ease on the reviewer.

Comment threadtests/integration/__init__.py
cassandra_version = Version(mcv_string)
except:
cassandra_version = Version('3.11.4')
cv_string = '3.11.4'

@vladzcloudiusvladzcloudiusMar 11, 2020

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

cv_string is not initialized outside this branch - looks wrong.

Comment threadtests/integration/__init__.py
Comment threadtests/integration/__init__.py
vladzcloudius
vladzcloudius suggested changes Mar 11, 2020
View reviewed changes
Comment threadcassandra/pool.py
@roydahan

roydahan commented Mar 26, 2020

Copy link
Copy Markdown

@fruch any progress with the testing of it?

ultrabug
ultrabug requested changes Mar 27, 2020
View reviewed changes

@ultrabugultrabug left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks a lot for working on this and sorry for the ridiculous amount of time it took me to review it

Comment threadcassandra/connection.py
Comment threadcassandra/connection.py
Comment threadcassandra/pool.py
Comment threadcassandra/pool.py
Comment threadcassandra/pool.py
Comment threadcassandra/pool.py
Comment threadcassandra/pool.py
Comment threadcassandra/pool.py
Comment threadcassandra/pool.py
Comment threadcassandra/pool.py
@fruch

fruch commented Mar 29, 2020

Copy link
Copy Markdown
OwnerAuthor

@fruch any progress with the testing of it?

We just have unittest and integration tests for it, did got to anything more then that.

@ultrabug

ultrabug commented Mar 29, 2020

Copy link
Copy Markdown
Collaborator

Once modifications will be done, I'll try to use https://github.com/numberly/cassandra-read-repairer on my staging cluster with this code!

@fruch

fruch commented Mar 30, 2020

Copy link
Copy Markdown
OwnerAuthor

Once modifications will be done, I'll try to use https://github.com/numberly/cassandra-read-repairer on my staging cluster with this code!

@ultrabug I've address all of you comment here:
scylladb#6

I would suggest you'll try using that PR branch

would be great to how it work on some real data

fruch added a commit that referenced this pull request Jun 1, 2020
@fruch
Merge pull request#1from guy9/adding_training_to_readme
27c1ce1
added training section to readme file
fruch pushed a commit that referenced this pull request Jun 3, 2020
@dgarcia360@fruch
Added GitHub Actions (#1)
67f3293
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ultrabugultrabugultrabug requested changes
+1 more reviewer
@vladzcloudiusvladzcloudiusvladzcloudius requested changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@fruch@roydahan@ultrabug@vladzcloudius