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

Implement query and get/put for example #67

Merged
merged 7 commits into from Jul 25, 2015

Conversation

Projects
None yet
3 participants
@Hoverbear
Copy link
Owner

Hoverbear commented Jul 22, 2015

This is in progress but I'll keep pushing up to this branch.

Work remaining:

  • Handling queries from a client on the server.
  • Determining if it's appropriate for query and proposal should have the same response variant. For discussion with @danburkert.

@Hoverbear Hoverbear referenced this pull request Jul 22, 2015

Closed

Proposals never return in a <3 node cluster #68

3 of 3 tasks complete
@Hoverbear

This comment has been minimized.

Copy link
Owner

Hoverbear commented Jul 22, 2015

I included a proof of bug for #68 in 4d42c30. Tests should be failing at the moment until the bug is solved.

@Hoverbear

This comment has been minimized.

Copy link
Owner

Hoverbear commented Jul 23, 2015

Ok, I think I'm finally done here. :)

panic!("unimplemented: waiting on changes to the Raft Client and StateMachine APIs");
fn get(args: &Args) {
let cluster = args.arg_server_address.iter()
.map(|v| {

This comment has been minimized.

@danburkert

danburkert Jul 25, 2015

Contributor

I think you can reduce this to .map(parse_addr).

panic!("unimplemented: waiting on changes to the Raft Client and StateMachine APIs");
fn put(args: &Args) {
let cluster = args.arg_server_address.iter()
.map(|v| {

This comment has been minimized.

@danburkert

danburkert Jul 25, 2015

Contributor

same here.

parse_addr(&v)
})
.collect();
// println!("{:?}", cluster);

This comment has been minimized.

@danburkert

danburkert Jul 25, 2015

Contributor

this looks leftover

// println!("{:?}", cluster);
let mut client = Client::new(cluster);
let response = client.propose(args.arg_new_value.as_bytes()).unwrap();
println!("{:?}", String::from_utf8(response))

This comment has been minimized.

@danburkert

danburkert Jul 25, 2015

Contributor

I think this should be println!("{}", String::from_utf8(response).unwrap())

// println!("{:?}", cluster);
let mut client = Client::new(cluster);
let response = client.query(args.arg_new_value.as_bytes()).unwrap();
println!("{:?}", String::from_utf8(response))

This comment has been minimized.

@danburkert

danburkert Jul 25, 2015

Contributor

println!("{}", String::from_utf8(response).unwrap())

parse_addr(&v)
})
.collect();
// println!("{:?}", cluster);

This comment has been minimized.

@danburkert

danburkert Jul 25, 2015

Contributor

this looks leftover

self.send_message(&mut message)
}

/// Queries an entry from the log. This is non-mutating and doesn't go through the durable log.

This comment has been minimized.

@danburkert

danburkert Jul 25, 2015

Contributor

This queries the state machine, not the log directly.

@@ -481,34 +490,69 @@ impl <S, M> Replica<S, M> where S: Store, M: StateMachine {
scoped_debug!("proposal from Client({})", from);

if self.is_candidate() || (self.is_follower() && self.follower_state.leader.is_none()) {
actions.client_messages.push((from.into(), messages::proposal_response_unknown_leader()));
scoped_debug!("proposal recieved, but leader is not known");

This comment has been minimized.

@danburkert

danburkert Jul 25, 2015

Contributor

misspelled received

} else if self.is_follower() {
scoped_debug!("proposal recieved, but replica is not leader");

This comment has been minimized.

@danburkert

danburkert Jul 25, 2015

Contributor

misspelled received

actions.client_messages.push((from, message));
} else {
scoped_debug!("proposal recieved, replica processing");

This comment has been minimized.

@danburkert

danburkert Jul 25, 2015

Contributor

misspelled received

This comment has been minimized.

@Hoverbear

Hoverbear Jul 25, 2015

Owner

Lol.... Thanks.

}
}
}
}

/// Issue a client query to the Raft replica.
fn query_request(&mut self,
from: ClientId,

This comment has been minimized.

@danburkert

danburkert Jul 25, 2015

Contributor

indent

from: ClientId,
request: query_request::Reader,
actions: &mut Actions) {
scoped_debug!("query from Client({})", from);

This comment has been minimized.

@danburkert

danburkert Jul 25, 2015

Contributor

this should probably be trace level

@danburkert

This comment has been minimized.

Copy link
Contributor

danburkert commented Jul 25, 2015

Overall looks really good, just a couple nits. Thanks for tracking down those bugs.

@Hoverbear Hoverbear force-pushed the replica_message branch from fb858a8 to 988fc4d Jul 25, 2015

@Hoverbear

This comment has been minimized.

Copy link
Owner

Hoverbear commented Jul 25, 2015

@danburkert rebased and fixed nits.

@Hoverbear Hoverbear referenced this pull request Jul 25, 2015

Closed

Renames? #70

@danburkert

This comment has been minimized.

Copy link
Contributor

danburkert commented Jul 25, 2015

@homu r+

@homu

This comment has been minimized.

Copy link
Collaborator

homu commented Jul 25, 2015

📌 Commit 988fc4d has been approved by danburkert

@homu

This comment has been minimized.

Copy link
Collaborator

homu commented Jul 25, 2015

⌛️ Testing commit 988fc4d with merge 6b31700...

homu added a commit that referenced this pull request Jul 25, 2015

Auto merge of #67 - Hoverbear:replica_message, r=danburkert
Implement query and get/put for example

This is in progress but I'll keep pushing up to this branch.

Work remaining:
* [x] Handling queries from a client on the server.
* [x] Determining if it's appropriate for `query` and `proposal` should have the same response variant. For discussion with @danburkert.
@homu

This comment has been minimized.

Copy link
Collaborator

homu commented Jul 25, 2015

☀️ Test successful - status

@homu homu merged commit 988fc4d into master Jul 25, 2015

3 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
homu Test successful
Details

@Hoverbear Hoverbear deleted the replica_message branch Jul 25, 2015

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