I would like to propose changing GetOmapValues and GetAllOmapValues methods of rados.IOContext to either not use a map to store key/value pairs or remove startAfter argument from those methods.
The problem with the current interface is the fact that Golang maps are naturally unordered. Thus, the method argument used for consequent pagination - startAfter - provides no value as the last object retrieved in the map iteration is non-deterministic. Sorting those values introduces an avoidable performance drawback. That can cause confusion with the provided interface in general.
My proposal is to either return an ordered slice of structs:
func (ioctx *IOContext) GetOmapValues(oid string, startAfter string, filterPrefix string, maxReturn int64) ([]struct{
key string
val string
}, error)or remove the startAfter argument from the method and refer to ListOmapValues method in docs for proper pagination:
func (ioctx *IOContext) GetOmapValues(oid string, filterPrefix string, maxReturn int64) (map[string][]byte, error)
I would prefer to remove the argument since it seems we already have an interface for the ordered pagination: ListOmapValues.
What do you think? Do I miss any usecases that would make more sense for the current API to stay? I will also happily create a PR for this change :)
I would like to propose changing
GetOmapValuesandGetAllOmapValuesmethods ofrados.IOContextto either not use a map to store key/value pairs or removestartAfterargument from those methods.The problem with the current interface is the fact that Golang maps are naturally unordered. Thus, the method argument used for consequent pagination -
startAfter- provides no value as the last object retrieved in the map iteration is non-deterministic. Sorting those values introduces an avoidable performance drawback. That can cause confusion with the provided interface in general.My proposal is to either return an ordered slice of structs:
or remove the startAfter argument from the method and refer to
ListOmapValuesmethod in docs for proper pagination:I would prefer to remove the argument since it seems we already have an interface for the ordered pagination:
ListOmapValues.What do you think? Do I miss any usecases that would make more sense for the current API to stay? I will also happily create a PR for this change :)