Skip to content

PyrakoonClient can use multiple clients to improve throughput in threaded situations (like the DAL)#215

Merged
JeffreyDevloo merged 5 commits into0.1.xfrom
0.1.x_pooled_pyrakoon_client
Mar 14, 2019
Merged

PyrakoonClient can use multiple clients to improve throughput in threaded situations (like the DAL)#215
JeffreyDevloo merged 5 commits into0.1.xfrom
0.1.x_pooled_pyrakoon_client

Conversation

@JeffreyDevloo
Copy link
Contributor

No description provided.

@JeffreyDevloo JeffreyDevloo merged commit aa3adc2 into 0.1.x Mar 14, 2019
@JeffreyDevloo JeffreyDevloo deleted the 0.1.x_pooled_pyrakoon_client branch March 14, 2019 12:20
@ghost ghost removed the state_inprogress label Mar 14, 2019
Package: openvstorage-extensions
Architecture: amd64
Pre-Depends: python (>= 2.7.2), python-paramiko, python-rpyc, python-protobuf, python-requests (>=2.9.1), python-ujson
Pre-Depends: python (>= 2.7.2), python-gevent (>= 0.13.0-1build2), python-paramiko, python-rpyc, python-protobuf,
Copy link
Member

Choose a reason for hiding this comment

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

no (< 3.0.0) ?

"""
raise NotImplementedError()

def set(self, key, value, transaction=None):
Copy link
Member

Choose a reason for hiding this comment

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

as stated in earlier discussions, I'm not a fan of the transaction = None) parameter, especially given that the parameter added here is a string. You might as well have the sequence as a parameter, or use the comfort calls on the sequence and remove this parameter altogether. With a sequence, it's absolutely clear no communication with the server is happening at all, and what the allowed set of scenarios is. Now it's less so.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Out of scope for the current changes really. It will require some refactor work

:return: The next key
:rtype: str
"""
encoding = 'ascii' # For future python 3 compatibility
Copy link
Member

Choose a reason for hiding this comment

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

isn't that an illusion?

array = array[:-1]
return str(array.decode(encoding))
array[index] = 0
return '\xff'
Copy link
Member

Choose a reason for hiding this comment

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

is this correct?

"""
Apply a transaction which is the result of the callback.
The callback should build the complete transaction again to handle the asserts. If the possible previous run was interrupted,
the Arakoon might only have partially applied all actions therefore all asserts must be re-evaluated
Copy link
Member

Choose a reason for hiding this comment

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

partially?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As in get interrupted mid sequence because of an assert and not do anything at all.
It's bad phrasing


_logger = Logger('extensions')

# Frequency at which the pool is populated at startup
Copy link
Member

Choose a reason for hiding this comment

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

at -> with

Add a new client to the pool
:return: None
"""
sleep_time = 0.1
Copy link
Member

Choose a reason for hiding this comment

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

what does this have to do with the SPAWN_FREQUENCY ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Its just a retry-interval

tries += 1
try:
transaction = transaction_callback() # type: str
return self.apply_transaction(transaction)
Copy link
Member

Choose a reason for hiding this comment

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

why have a succes variable if you never set it to True ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants