Feat/leader election #2
Conversation
| import raftPb "github.com/adylanrff/raft-algorithm/proto/raft" | ||
|
|
||
| type RequestVoteRequestDTO struct { | ||
| *raftPb.RequestVoteRequest |
There was a problem hiding this comment.
gw personally bukan fans dto style di java. dan menurut gw pribadi gak cocok aja kalau dipakai di go.
yang di generate dari raft.proto itu kan sebenernya api object ya. IMO, di api internal kita bisa lgsg pakai ini instead of ngewrap dengan another DTO lagi.
| }).Error("already voted before") | ||
| return model.NewRequestVoteResponseDTO(r.state.CurrentTerm, false), nil | ||
| } | ||
|
|
There was a problem hiding this comment.
ada election restriction yang masih kurang mestinya disini. mestinya servernya juga ensure log mana yang lebih up-to-date given the last log index and term. but, given that we haven't done anything with the log yet, i think we can add it later.
There was a problem hiding this comment.
Oh yes bener, kemarin skip dulu karena lognya belum ada isinya haha
raft/raft.go
Outdated
| if voteResp.VoteGranted { | ||
| atomic.AddInt32(&votes, 1) | ||
| } | ||
| if voteSeen == len(r.config.ClusterMemberAddreses)-1 { |
There was a problem hiding this comment.
ini buat ensure semua servernya mesti ngasih respon vote dulu? in case salah satu servernya ngasih response terlalu lama meskipun majority vote udah setuju, ini gak bakal ngeblock election process kah?
There was a problem hiding this comment.
Ah bener, harusnya nggak perlu wait until semua node ngevote dulu. thanks will fix
| @@ -0,0 +1,42 @@ | |||
| package rpc | |||
There was a problem hiding this comment.
gw pikir proto, rpc and util package ini gak perlu jadi package yg berbeda. it can be part of raft package.
kalau mau tetap dipisah, better to do something like rpc.NewClient() instead of rpc.NewRPCClient(). Ketika NewClient dipanggil dari package rpc, udah ada context tambahan bahwa itu adalah client rpc.
tapi kalau digabung, bisa lebih sederhana di dalam package raft.
type Raft struct {
rpc RPCClient
}
func NewRaft() {
r := &Raft{
rpc: newRPCClient(),
}
}
newRPCClient() bisa private utk package raft. jadi bisa naikin module cohession, dan ngurangin coupling ke package lain. lagi pula yg akan pakai rpc client cuma module raft aja kan sebenernya.
There was a problem hiding this comment.
thanks feedbacknya. setuju. tapi untuk yang 'client', soalnya gue kepikirannya nanti ada client lain untuk usecase lain yang pakai ini (e.g. untuk call leader untuk nambahin log)
raft/raft.go
Outdated
| for range r.heartbeatTicker.C { | ||
| for _, memberAddress := range r.config.ClusterMemberAddreses { | ||
| if memberAddress != r.address { | ||
| go r.raftClient.AppendEntries(memberAddress, model.NewAppendEntriesRequestDTO()) |
There was a problem hiding this comment.
cmiiw. sepahaman gw heartbeat ini cuma perlu utk kondisi idle ketika gak ada entries dari client, karena ketika ada write dari client, itu juga bisa dipakai sebagai pengganti healthcheck
There was a problem hiding this comment.
hmm kalo dari visualisasi di https://raft.github.io kok kayaknya itu tetep kirim heartbeat ya?
There was a problem hiding this comment.
sebenernya agak susah ngeobserve ini dari visualisasi. tapi kalau requestnya clientnya cukup frequent, sebenernya heartbeat jadi gak perlu. karena payload append entries rpc utk ngecommit entry yg baru masuk tetep bisa dipake sama receivernya.
Dari paper raft:
Upon election: send initial empty AppendEntries RPCs
(heartbeat) to each server; repeat during idle periods to
prevent election timeouts (§5.2)
There was a problem hiding this comment.
oh right
jadi paling bikin idle timeout aja ya instead of heartbeat
thanks2
No description provided.