Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ This is due to an inherently bad approach to controversial interfaces.
BUG FIXES:
* Router.Call bugfix: set destinationName := rs.info.Name if destination exists.
* Router.Route bugfix: handle outdated *Replicaset object (resolve issue #11).
* DiscoveryHandleBuckets: identify replicasets by name for logging purpose.

FEATURES:
* Now when calling RemoveInstance, if an empty replicaset name is passed, the replicaset will be calculated automatically.
Expand Down
52 changes: 36 additions & 16 deletions discovery.go
Original file line number Diff line number Diff line change
Expand Up @@ -191,34 +191,54 @@ func (r *Router) bucketSearchBatched(ctx context.Context, bucketIDToFind uint64)
// DiscoveryHandleBuckets arrange downloaded buckets to the route map so as they reference a given replicaset.
func (r *Router) DiscoveryHandleBuckets(ctx context.Context, rs *Replicaset, buckets []uint64) {
routeMap := r.getRouteMap()
removedFrom := make(map[*Replicaset]int)
removedFrom := make(map[string]int)

var newRsName string
if rs != nil {
newRsName = rs.info.Name
}

for _, bucketID := range buckets {
// We don't validate bucketID intentionally, because we can't return an error here using the current API.
// We can't silence this error too, the caller should know about invalid arguments.
// So the only way to inform the caller is to panic here,
// that should happen if bucketID is out of the array bounds.
// if bucketID < 1 || r.cfg.TotalBucketCount < bucketID {
// continue
// }

oldRs := routeMap[bucketID].Swap(rs)

if oldRs == rs {
continue
var oldRsName string
if oldRs != nil {
oldRsName = oldRs.info.Name
}

// NOTE: oldRs and rs might have the same name, we intentionally don't check this case to keep the logic simple

// We don't check oldRs for nil here, because it's a valid key too (if rs == nil, it means removed from unknown buckets set)
removedFrom[oldRs]++
if oldRsName != newRsName {
// Something has changed.
// NOTE: We don't check oldRsName for empty string ("") here,
// because it's a valid key too (it means bucketID became known)
removedFrom[oldRsName]++
}
}

var addedToRs int
for rs, removedFromRs := range removedFrom {
addedToRs += removedFromRs
var addedToNewRsTotal int
for removedFromRsName, removedFromCount := range removedFrom {
addedToNewRsTotal += removedFromCount

switch rs {
case nil:
r.log().Debugf(ctx, "Added new %d buckets to the cluster map", removedFromRs)
default:
r.log().Debugf(ctx, "Removed %d buckets from replicaset %s", removedFromRs, rs.info.Name)
if removedFromRsName == "" {
// newRsName cannot be an empty string here due to the previous for-loop above.
r.log().Debugf(ctx, "Added new %d buckets to the cluster map", removedFromCount)
} else {
r.log().Debugf(ctx, "Removed %d buckets from replicaset '%s'", removedFromCount, removedFromRsName)
}
}

r.log().Infof(ctx, "Added %d buckets to replicaset %s", addedToRs, rs.info.Name)
if newRsName == "" {
r.log().Infof(ctx, "Removed %d buckets from the cluster map", addedToNewRsTotal)
} else {
r.log().Infof(ctx, "Added %d buckets to replicaset '%s'", addedToNewRsTotal, newRsName)
}
}

func (r *Router) DiscoveryAllBuckets(ctx context.Context) error {
Expand Down
Loading