Community Roadmap/Planning Meetings?
Christopher Sterling
Divjot Arora
Christopher Sterling
Sorry for the late reply Divjot -
Took me a while to get all this together. Sorry for the long post - I was trying to be thorough:
My general grievance with mongo-go-driver is how much harder it is to do normal day-to-day queries compared to mgo. I think the best way to understand that is to see it. Here are a few standard operations we end up doing implemented as best I can in mgo versus mongo-go-driver.
Update an existing object by it’s ID - one of the most common operations we perform in order to keep our performance optimal:
mgo:
func MgoUpdateID() error {
coll := mgo.Collection{}
var obj someStruct
err := coll.UpdateId(obj.ID, obj)
if err == mgo.ErrNotFound {
logrus.Warn("Couldn't find that")
} else if err != nil {
logrus.Error("There was an internal issue with the DB")
}
return err
}
mongo-go-driver:
func MongoGoDriverUpsert() error {
coll := mongo.Collection{}
var obj someStruct
info, err := coll.UpdateOne(
context.Background(),
bson.M{"_id": obj.ID}, obj)
if err == nil {
// err no longer is raised if not found - must compute from info result
if info.MatchedCount == 0 &&
info.ModifiedCount == 0 &&
info.UpsertedCount == 0 {
err = db.ErrNotFound
}
}
if err == db.ErrNotFound {
logrus.Warn("Couldn't find that")
} else if err != nil {
logrus.Error("There was an internal issue with the DB")
}
return err
}
Note that I now have to explicitly create a context, create the ID filter bson query, check the info result to generate a NotFound error (to make the error behavior consistent - NotFound is currently raised by FindOne but not by any other function). It’s more complicated to use now. It’s easy for a dev to forget the NotFound behavior is now inconsistent - it’s easier to introduce a bug. A majority of this code has to be copy+pasted over and over in each function. It makes me want to write a wrapper library for ease-of-use, but I feel like it makes more sense to just work on integrating the changes into mongo-go-driver itself.
Another example that I think shows the differences in usability comes in at Distinct. Here’s an example with a sort and a limit:
Mgo:
func MgoDistinctLimitSort(limit int) error {
coll := mgo.Collection{}
var adminNames []string
err := coll.Find(bson.M{}).
Sort("-adminName").
Limit(limit).
Distinct("adminName", &adminNames)
if err == mgo.ErrNotFound {
logrus.Warn("Couldn't find any distinct values")
} else if err != nil {
logrus.Error("There was an internal issue with the DB")
}
return err
}
Mongo-go-driver:
func MongoGoDriverDistinctLimitSort(limit int) error {
coll := mongo.Collection{}
adminNamesIface, err := coll.Distinct(
context.Background(),
"adminName",
bson.M{})
if err == nil && len(adminNamesIface) == 0 {
// Pseudo-injection of not found error
err = db.ErrNotFound
logrus.Warn("Couldn't find any distinct values")
return err
} else if err != nil {
logrus.Error("There was an internal issue with the DB")
return err
}
adminNames := make([]string, len(adminNamesIface))
for i, admin := range adminNamesIface {
if adminName, ok := admin.(string); ok {
adminNames[i] = adminName
} else {
err = fmt.Errorf("The adminName '%#v' could not be coerced to a string", admin)
break
}
}
// There is no ability to set Sort or Limit on Distinct options
sort.Reverse(sort.Strings(adminNames)) // Sort by alpha
if limit > 0 && len(adminNames) > limit {
adminNames = adminNames[0:limit] // Limit
}
return err
}
Last example - Find using an iterator and timeout:
mgo:
func MgoFindWithIterTimeout(limit int) error {
someProcessedString := ""
var someObj someStruct
var err error
coll := mgo.Collection{}
iter := coll.Find(bson.M{}).Limit(limit).Tail(
time.Duration(2) * time.Second)
defer iter.Close()
for iter.Next(&someObj) {
someProcessedString += someObj.ID.String()
}
if iter.Timeout() {
logrus.Error("The DB timed out")
return fmt.Errorf("db timeout")
}
err = iter.Err()
if err == mgo.ErrNotFound {
logrus.Warn("Couldn't find any distinct values")
} else if err != nil {
logrus.Error("There was an internal issue with the DB")
}
return err
}
mongo-go-driver:
func MongoGoDriverFindWithIterTimeout(limit int) error {
someProcessedString := ""
var someObj someStruct
var err error
coll := mongo.Collection{}
ctx, cancel := context.WithTimeout(
context.Background(),
time.Duration(2)*time.Second)
defer cancel()
cursor, err := coll.Find(ctx, bson.M{})
defer cursor.Close(context.Background())
for cursor.Next(context.Background()) {
if err = cursor.Decode(&someObj); err != nil {
break
}
someProcessedString += someObj.ID.String()
}
if err == nil {
err = cursor.Err()
}
if err == context.DeadlineExceeded {
// A timeout occurred
logrus.Error("There was a timeout")
return err
} else if err == nil && len(someProcessedString) == 0 {
err = db.ErrNotFound
logrus.Warn("Couldn't find any distinct values")
return err
} else if err != nil {
logrus.Error("There was an internal issue with the DB")
return err
}
return err
}
As a dev, I don’t really care about the context. Especially since mongo isn’t currently honoring it when it’s canceled. All I want to do is provide a time Duration and let the library handle the timeout internally. I don’t want to have to check for ErrNotFound.
Here is a goplay of my mongo-go-driver examples: https://goplay.space/#oYo-txFAmfC
And a goplay of the mgo examples: https://goplay.space/#v4QFP_ZrP_e
In addition to the ease-of-use issues, the migration path for us was insanely hard. We thoroughly read https://www.mongodb.com/blog/post/go-migration-guide but there were SO MANY gotchas that we had to account for that aren’t mentioned by any documentation. Most of these things required us to read the mongo-go-driver codebase directly to solve. e.g.
The “truncate” tag is required with float32 for our pre-existing date mgo seemed to natively store them as float64. There was no documentation on the truncate tag. The error message the driver yields is unhelpful to actually know what to do to fix the issue.
Uninitialized arrays are handled very differently under the covers - mgo handles uninitialized slices as empty arrays, this makes things like $push operations really easy later. With mongo-go-driver, uninitialized arrays are set to null, which means that any subsequent $push or other array operations will fail. Making this a toggleable option, something like `SetHandleUninitializedSlicesAsArrays` would be helpful. The decoder/encoder are really hard to use directly, so adding some ease of use functions around these common options would be helpful.
Timestamps are handled differently under the covers - used to, if they were a string they were coerced into a time.Time. Now there is no coercion, if the timestamp isn’t a datetime in mongo, it errors out, even though golang natively supports parsing the string timestamp.
ErrNotFound is handled completely differently than mgo. It is inconsistent - only raised by FindOne and not raised by any other call. Adding something to writeconcern, such as `SetRaiseErrorOnNoDocuments(true)` could be used to make this behavior consistent. I realize this isn’t supported by the API, but it doesn’t seem like it would be difficult to implement natively in golang.
The use of bson.D was really confusing in the docs/examples. Why use it rather than bson.M and bson.A? They result in much cleaner looking code in general.
ObjectID is now on it’s own island in primitives - you have to import bson for the standard types and also import primitives just to pick up ObjectID.
For the 2nd and 3rd points it took a lot of work to get any kind of help. Your responses on the Google Group have been invaluable, but I’m still having issues implementing the solutions you suggested (e.g. using .Kind() to override default nil slice behavior and using the registry to modify default handling of datetimes).
There are not JIRA issues for all of my grievances, but here are some JIRA Issues that relate (some open/some closed):
https://jira.mongodb.org/browse/GODRIVER-1441 - DeleteOne does not return any useful information when no document exists
https://jira.mongodb.org/browse/GODRIVER-549 - Add ObjectID struct tag for strings
https://jira.mongodb.org/browse/GODRIVER-1433 - Add RespectNilValues equivalent for the mgo registry
https://jira.mongodb.org/browse/GODRIVER-707 and https://jira.mongodb.org/browse/GODRIVER-972 - Support for IsDup()
Without this helper, we had to implement our own version - no idea if it’s right or not - we basically ripped off the code from mgo
https://jira.mongodb.org/browse/GODRIVER-988 - Document decoding to empty interface
https://jira.mongodb.org/browse/GODRIVER-964 - Make option type construction consistent with the options package (e.g. readpref, readconcern, writeconcern)
https://jira.mongodb.org/browse/GODRIVER-1419 - Add Collection.FindAll
https://jira.mongodb.org/browse/GODRIVER-749 - Add support for findAndModify
https://jira.mongodb.org/browse/GODRIVER-1332 - primitive.ObjectID json omitempty doesn't work
https://jira.mongodb.org/browse/GODRIVER-1214 and https://jira.mongodb.org/browse/DRIVERS-555 - Cancelling the context associated with a query does not actually cancel the query
https://jira.mongodb.org/browse/GODRIVER-1117 - Allow marshaling custom interfaces inside arrays or structs
https://jira.mongodb.org/browse/GODRIVER-1075 - Semi-related to ErrNotFound -
https://jira.mongodb.org/browse/GODRIVER-582 - ensureID should return consistent types
Ideally, an array of bson.ObjectIDs would be returned rather than interfaces that have to be casted
https://jira.mongodb.org/browse/GODRIVER-1363 - InsertMany returns InsertedID's from insertions that errored
Generally*, the closed/won’t work tickets seemed to read as “If mongo DB doesn’t do natively / can’t be supported by the API, it won’t be worked”. Is it frowned upon to have extra processing in the golang library in the mongo org? From a standardization perspective, I suppose I can understand this attitude, but it results in a bad experience for programmers that want to make DRY code without having to roll their own wrappers for each project.
I’m more than happy to take on writing an “easy-mongo” wrapper if the team is willing to consider allowing it to be part of the library. I’m not looking to fork and maintain compatibility - I think that’s why mgo died. I would like this to live in mongo-go-driver. It would include U/X things, like adding *ByID() functions, handling timeout/contexts all under the covers, add the ability to make ErrNotFound easy to raise consistently, improving ease of use with Distinct(), adding common encoders/decoders for nil slice handling/timestamps/float truncation/etc. Generally making the code as lean as possible to make it fast to implement new functionality.
Thoughts on all of this?
Rachelle Palmer
Christopher Sterling
Stennie Steneker
has this been migrated to the new forum? I was having a hard time locating it, but maybe I'm just searching for it wrong.