-
Notifications
You must be signed in to change notification settings - Fork 33
Description
The function vshard.router.routeall
is not marked as internal API (like it's done for vshard.router.bucket_discovery
e.g.), but it returns router.replicasets
table, which is used by the router itlself. If user modifies the returned from routeall
table (e.g. deletes some replicaset just because he wants to skip it during request), then router is broken and cannot continue to work properly after that. Moreover, these types of errors are hard to find.
The same applies to vshard.router.route
and vshard.router.bucket_discovery
(internal one) functions.
Proposed solution
Create a special table of replicasets during router configuration, so that not to recreate it every time when we need to return replicasets to user. This table doesn't allow inserting of a new value or deleting of the existing one (__newindex
), as otherwise the change of that table will be preserved between routeall
.
Every replicaset is wrapped (it may be returned as value in routeall
table or from route
): replicaset has only one real field: _replicaset
, which is the one we return now. It has all of the supported replicaset functions in the metatable.
This way user won't be able to modify the fields of returned tables (if he won't access rs._replicaset
, which he should not if he doesn't understand, what he's doing). But the table is RO, which may not be very convenient (see alternative).
Considered alternatives
table.deepcopy
table.copy
or table.deepcopy
may seem like a simple solution, but it's not. Connections cannot be copied. User will have to reconnect for every request after every routeall
in such case.
The alternative solution is to create the wrapper on every routeall
or route
call and to wrap the replicaset on every route
call. This way, user will be able to modify the returned by routeall
or route
table as he likes, we won't forbid adding new fields or deleting the old ones. It makes user experience less painful but requires creating it on every call (is it that costly? @Gerold103).
- Allow to access individual fields without modification of the router and replicas
However, user usually needs to access fields of the replicaset (e.g. bucket_count
) and we don't want user to end up accessing _replicaset
field. For that we modify the __index
of the replicaset wrapper, allowing to access safe fields: all fields, except master_cond
, on_master_required
(they can be overwritten). It's forbidden to add new fields (or change _replicaset
) to replicaset wrapper.
In that wrapper user still needs to access replica objects. So, it must also be wrapped. Still, all functions of the replica's metatable; only _replica
real field, all internal fields can be accessed via __index
, except conn
. Link in replica next_by_priority
points to the wrapper. master
and replica
in replicaset wrapper point to replica wrappers. priority_list
and replicas
in replicaset wrapper are the tables of wrappers, it's forbidden to modify them. It's forbidden to add new fields to replica wrapper.
It was decided to allow smth only if smb asks!