Debugging and backtracking the error, I narrowed the error to a change introduced in Mongo-Core-HolgerHansPeterFreyther.44. What I found is that the new implementation of MongoDatabase>>addCollection: no longer tolerates adding an already existing collection.

This is how adding a collection works, before and after:

• before (directly in MongoDatabase>>addCollection:)

  | command reply |
  command := Dictionary new.
  command at: 'create' put: aString.
  reply := self command: command.
  reply at: 'ok' ifAbsent: [self halt].
  ^MongoCollection database: self name: aString

• currently (delegated to MongoDatabase>>addCollection:capped:size:max:)

  | command reply |
  command := SmallDictionary new.
  command at: 'create' put: aString.
  aCapped ifTrue: [
  command at: 'capped' put: true.
  aSize ifNotNil: [command at: 'size' put: aSize].
  aMax ifNotNil: [command at: 'max' put: aMax]].
  reply := self command: command.
  (reply at: 'ok' ifAbsent: [self halt]) = 1.0 ifFalse: [
  ^self error: 'Command failed'.
  ].
  ^MongoCollection database: self name: aString

I didn't analyse why the tests add an existing collection. So not sure if it's the test or the code that should be fixed.

But, I tried inserting the following code to tolerate adding an existing collection, and it worked fine:

(reply at: 'ok' ifAbsent: [self halt]) = 1.0 ifFalse: [     
    "Tolerate error 48: collection already exists"
    ((reply at: 'code') = 48) ifFalse: [ ^self error: 'Command failed' ]
].

The code is not nice, but I can commit it if you agree. No idea why Travis builds don't complain on this, but I reproduced the hang on my linux in Pharo 3, 4 and 5.

Opinions?

Hi,

The class VOLazyProxy has this method:

becomeForwardKeepingHash: otherObject
    "Primitive. All variables in the entire system that used to point to the receiver now point to the argument.
    If copyHash is true, the argument's identity hash bits will be set to those of the receiver.
    Fails if either argument is a SmallInteger."

    (Array with: self)
        elementsForwardIdentityTo: (Array with: otherObject)
        copyHash: false

Become is a delicate feature whose details sometimes I don't grasp... so I wonder if this method isn't buggy, since "keeping hash" in the selectors sounds like "copyHash" parameter should be true.

Thanks.

removeValue: is not removing version, so it grows indefinitely.

this is working on master branch, but needs to be backported to 1.3.1 version

Didn't check in code nor proper mailing-lists, but report anyway:

ByteSymbol(Object)>>doesNotUnderstand: #greaseString
MASelectorAccessor>>selector:
MASelectorAccessor class>>selector:
ByteSymbol(Symbol)>>asAccessor
VOMongoShadowDescription(MADescription)>>accessor:
VOTestPilot class>>descriptionShadowKeywords
CompiledMethod>>valueWithReceiver:arguments:
VODescriptionBuilder>>build:in:
[ :each | self build: each in: aClass ] in VODescriptionBuilder>>buildDescriptionsFor:inContainer: in Block: [ :each | self build: each in: aClass ]
Array(SequenceableCollection)>>collect:
VODescriptionBuilder>>buildDescriptionsFor:inContainer:
VODescriptionBuilder>>buildClass:
VODescriptionBuilder>>build:
[ self build: aClass ] in VODescriptionBuilder>>for: in Block: [ self build: aClass ]
[ self at: key put: aBlock value ] in Dictionary>>at:ifAbsentPut: in Block: [ self at: key put: aBlock value ]
Dictionary>>at:ifAbsent:
Dictionary>>at:ifAbsentPut:
VODescriptionBuilder>>for:
VOMongoRepositoryResolver>>collectionNameFor:
VOMongoRepositoryResolver>>collectionAt:inDatabase:
[ :db | (self collectionAt: aClass inDatabase: db) size ] in VOMongoRepositoryResolver>>basicCount: in Block: [ :db | (self collectionAt: aClass inDatabase: db)...etc...
[ ^ aBlock value: db ] in VOMongoSessionPool>>withDatabase: in Block: [ ^ aBlock value: db ]
BlockClosure>>ensure:
VOMongoSessionPool>>withDatabase:
VOMongoRepositoryResolver>>basicCount:
[ ^ self basicCount: aClass ] in VOMongoRepositoryResolver>>count: in Block: [ ^ self basicCount: aClass ]
BlockClosure>>on:do:
VOMongoExecuteStrategy>>execute

Extracted from this job

Hi. The following code reproduces the issue. First, it saves an Owner which has a Dog, then updates the Owner's name in a forked image, then checks the pet's owner name in the original image and shows it's the old name. Finally, the code forces the update of the Owner and checks the name is updated.

Code:

"load Voyage+ImageWorker"
Metacello new 
    repository: 'github://estebanlm/voyage/mc';
    baseline: 'VoyageMongo';
    load.
Metacello new 
    smalltalkhubUser: 'PharoExtras' 
    project: 'ImageWorker';
    configuration: 'ImageWorker';
    version: #development;
    load.

"set up db"
repository := VOMongoRepository database: 'indirect-objects'.
repository enableSingleton.

"set up scenario"
repository removeAll: VOTestOwner.
repository removeAll: VOTestDog.
50 milliSeconds asDelay wait.
dog := VOTestDog new name: 'carola'; yourself.
owner := 
    VOTestOwner new 
        name: 'foo'; 
        addPet: dog;
        yourself.
repository save: owner.
dogId := repository idOf: dog.

"clean up"
dog := nil.
3 timesRepeat: [Smalltalk garbageCollect].

"check owner name is foo"
50 milliSeconds asDelay wait.
dog := repository selectOne: VOTestDog where: (Dictionary with: '_id' -> dogId).
('name before: ', dog owner name) logCr.

"clean up"
dog := nil.
3 timesRepeat: [Smalltalk garbageCollect].

"FORKED IMAGE: change owner name to bar"
ImageWorker 
    do: [ 
        (repository selectAll: VOTestOwner) 
            do: [ :each | each name: 'bar'. repository save: each ].
         'change to bar' logCr ]
    within: 3 seconds.

"check owner name is bar"
50 milliSeconds asDelay wait.
dog := repository selectOne: VOTestDog where: (Dictionary with: '_id' -> dogId).
('name after change: ', dog owner name) logCr.

"check owner name after forcing update"
((repository selectAll: VOTestOwner) collect: #name as: Array) logCr.
('name after forced update: ', dog owner name) logCr.

The Transcript prints:

'name before: foo'
'name after change: foo'
#('bar')
'name after forced update: bar'

because of copying hash when solving VOLazyProxy

As discussed some tome ago in a pharo-dev's thread, the implementation of newVersion is wrong, making VOMongoRepository's save: and update: operations slow.

This is the code:

VOMongoRepositoryResolver>>
newVersion 
    ^UUIDGenerator new makeSeed

With every run the number open connections grow.

Some key core classes reference VOMongoDescriptionBuilder, VOMongoSerializer and VOMongoMaterializer. The main problem is that VOMongoDescriptionBuilder needs Magritte.

As a result... extracted from travis:
VOMongoRepositoryResolver>>futureWithNewVersion:do: (VOMongoSerializer is Undeclared) VOMongoRepositoryResolver>>retrieveClassOf:json: (VOMongoSerializer is Undeclared) VOMongoRepositoryResolver>>retrieveObjectOf:json: (VOMongoSerializer is Undeclared) VOMongoRepository>>descriptionBuilder (VOMongoDescriptionBuilder is Undeclared) VOMongoRepository>>materializer (VOMongoMaterializer is Undeclared) VOMongoRepository>>serializer (VOMongoSerializer is Undeclared)

Isn't Magritte central enough in Voyage to merge these packages in one?

It smells to unused. But I didn't check in other packages than the ones in default Baseline group.

VOMongoRepository>>
commit: aBlock 
    aBlock value.

and

VOReposutory>>
root: anObject commit: aBlock
    "Specifies a root object, to perform better transactions. 
     In many cases (as in memory repository, or relational based repository) this is the same as plain #commit:"
    self commit: aBlock

(reported by Sabine)

on mongo console, this brings me a persons information from mongo db
db.Persons.find({"_id" : ObjectId("75b900000000000000000000")})

I try to query the same person from pharo and I do not succeed.

This makes my image hanging:
RKAPerson selectMany: {('_id' -> (OID value:'75b900000000000000000000'))}

also other variations e.g.
RKAPerson selectMany: {('_id' -> ('75b900000000000000000000')). } asDictionary.

do not work

Other queries like
RKAPerson selectMany:{('lastName' -> ('Maier')).} asDictionary.
or
RKAPerson selectMany:{('nextTripNumber' -> (57)).} asDictionary.

work fine.

How can I query for an object with the object id?

check this thread: http://forum.world.st/Mongo-find-object-by-id-td4842231.html

Not sure how important is this issue, but we could fix it.

*** Warning: Warning: This package depends on the following classes: TimeStamp You must resolve these dependencies before you will be able to load these definitions: TimeStamp>>#bsonTypeCode TimeStamp>>#writeBSONOn:

In a PR's discussion, @zecke said:

Again you can use >>#command: with getLastError and force a sync? Then the last write/delete/op must have succeeded.

referred to mc/Voyage-Mongo-Tests.package/VOMongoTest.class/instance/waitForWriteOperation.st

Unused (only 1 sender and 1 implementor... itself)

rawSelectMany: aClass where: aDictionary
    ^resolver rawSelectMany: aClass where: aDictionary

There are no tests using these two methods, so I'm not sure what the semantics are supposed to be ... For GemStone I do a db commit during VORepository>>save:, so perhaps VORepository>>commit: and VORepository>>root:commit: aren't really necessary? Or perhaps I'm missing something else?

Voyage can take advantage of replication, an important feature in mongodb: https://docs.mongodb.org/manual/core/replication-introduction/

I want to refine this issue in more concrete terms. For the moment, I just paste a small replica-set demo: The following are commands to create a test replica set and test it (generated from https://docs.mongodb.org/manual/tutorial/deploy-replica-set-for-testing/)

# terminal 1
mongod --port 27030 --dbpath some_path/testreplicationdb/rs0-0 --replSet rs0 --smallfiles --oplogSize 128

# terminal 2
mongod --port 27031 --dbpath some_path/testreplicationdb/rs0-1 --replSet rs0 --smallfiles --oplogSize 128

# terminal 3
mongod --port 27032 --dbpath some_path/testreplicationdb/rs0-2 --replSet rs0 --smallfiles --oplogSize 128

# terminal 4

# log in to primary
mongo --port 27030 
rs.initiate() # make primary
rs.conf() #just to check
rs.add("my_host:27031")
rs.add("my_host:27032")
rs.status()  #just to check
# add some data to primary
db.categories.insert( { _id: "MongoDB", parent: "Databases" } )
db.categories.insert( { _id: "Languages", parent: "Programming" } )

#log in to a secondary
mongo --port 27031 
rs.slaveOk() #make readable
db.categories.find() # gets:
{ "_id" : "MongoDB", "parent" : "Databases" }
{ "_id" : "Languages", "parent" : "Programming" }

For a unit test I have inserted one record and I know there is only one and would like to retrieve it. My unit test is using VOMemoryRepository and I do:

res := repository selectOne: BlaClass where: [:each | true].
self assert: res...

If I do the same with the VOMongoRepository then I get

Can't canonicalize query: BadValue: unknown top level operator: $query

inside the $err string. It might be the easiest to add a selectAny to VOMongoRepository/VOMemoryRepository or can you think of something else?

2 failures and 8 errors out of 16 tests!

There are more than 5 occurences that can be replaced by VOMongoJSONSerializer fieldRootIdor whatever is the right way to access this string.

VOMongoRepository has a forceEager instance variable with accessor and mutator, but they are not referenced/sent.

Should we remove this property from API? or should it be implemented?

when selecting some data with voyage version 1.4.1 from mongo, I geht MNU VOMongoToManyDescription>>jsonName. I assume this was earlier mongoName and is not yet implemented. mongoName instead of jsonName works

Both in class-side:

objectClass: aClass id: anObject
    ^self repository: nil objectClass: aClass id: anObject

and:

classFor: aClass
    "Make a subclass of myself that is large enough to hold an instance of aClass.
    This makes becomeForward: resolution much faster, due to a fast path which simply memcpy's.
    For variably sized or compact classes, this is sadly not possible, and we need Spur for fast proxys."

    | bogusSlots |
    ^ (aClass instSpec = self instSpec
        and: [ 
            aClass layout compactClassIndex = self layout compactClassIndex
                and: [ (bogusSlots := aClass layout fieldSize - self layout fieldSize) > 0 ] ])
        ifFalse: [ self ]
        ifTrue: [ |shapedClass|
            shapedClass := 
            (AnonymousClassInstaller
                make: [ :builder | 
                    builder
                        superclass: self;
                        layoutClass: self layout class;
                        slots: (aClass layout allVisibleSlots last: bogusSlots) ]).
            shapedClass methodDict at: #becomeForward: put: self >> #becomeForwardWithSameShape:.
            shapedClass  ]

Last the build of magritte3 has been changed so that pharo includes the Glamour support in the default group. That means that even Glamour Core is loaded into the image which is not wanted in the server image case

But MongoMockTests are all red. What should we do with them?

They are supposed to run in an externally configured db, no? do you know how to configure it? some script around?

This started in #56 where Sabine had problems due to the changes in class and method names in the big refactoring. We could collect here some changes that can be updated as once.

Need to check:

VOMongo_Description changes to VO_Description as the descriptions are not mongo related

Since Pharo-4.0 and Pharo-3.0 haven't been built with a relatively recent version of Metacello the tag pattern matching feature is not present and causes build failures here and here ...

Presumably support for Pharo3.0 and Pharo4.0 will be frozen at some point in time anyway, so you might just wire in the tag value for those two platforms (presuming that the version of Metacello installed by default even supports tags ... I think it would) and use the pattern matching for pharo5.0 and beyond

As we all know referring to a symbolic version does not work, be it stable or development. In git parlance this would be the master brach which is the default if only specifying the repository. I think it is unavoidable to have deliberate version updates of mongotalk and all dependent projects.
So I propose to release a new version of mongotalk and pin the voyage configuration to that version until there is an update of mongotalk we want to integrate. Any thoughts?

Hi Esteban,
first thanks for your work - maybe I found an issue, here is what I've done:

loaded a fresh Pharo 4.0 image
loaded VoyageMongo via Configuration Browser (Install Stable Version)
opened up a Playground and did (via the green 'play' button):

|repository|

repository := VOMongoRepository
host:'hostOn.mongolab.com'
port: 43158
database:'databasename'
username: 'username'
password: 'password'.

repository enableSingleton.

repository inspect.

This leads to an error which seems to be connected to 'gtInspectorMagritteIn:' which was updated in the package Magritte-GT on 03.06.2015. (no instance 'repository' is going to be created).

Please excuse my bad analysis on this, because I have only very limited skills.

Best regards

Marcus

Damien reports this problem:

I'm following the tutorial at
https://ci.inria.fr/pharo-contribution/job/EnterprisePharoBook/lastSuccessfulBuild/artifact/Voyage/Voyage.html.

In Sections "2.2. Embedding objects" and "2.3. Referencing other roots",
the tutorial says:

1. add #isVoyageRoot to Rectangle and Point
2. create a rectangle with "aRectangle := 42@1 corner: 10@20."
3. send #save to the rectangle

In mongo, this results in having:

show collections
Associations
Rectangle
false
system.indexes

db.Rectangle.find()
{ "_id" : ObjectId("d710db9b4b673b0718000001"), "#instanceOf" : "Rectangle", "#version" : 856503121, "corner" : { "#collection" : "false", "#instanceOf" : "Point", "__id" : ObjectId("d710dc994b673b0718000003") }, "origin" : { "#collection" : "false", "#instanceOf" : "Point", "__id" : ObjectId("d710dc994b673b0718000002") } }

db.false.find()
{ "_id" : ObjectId("d710dc994b673b0718000002"), "#version" : NumberLong("3790997929"), "#instanceOf" : "Point", "x" : 10, "y" : 1 }
{ "_id" : ObjectId("d710dc994b673b0718000003"), "#version" : 982787913, "#instanceOf" : "Point", "x" : 42, "y" : 20 }

So, no "Point" collection, and a "false" collection instead.

"Selecting instances of aClass should be done in the mongo query"

I only copy what's in the code:

basicSelectMany: aClass where: aDictionary limit: limit offset: offset
    "Selecting instances of aClass should be done in the mongo query, not here"
    self flag: #todo.

    ^((self basicRawSelectMany:  aClass where: aDictionary limit: limit offset: offset)
        collect: [ :each | self retrieveObjectOf: aClass json: each ]
        as: repository collectionClass)
            select: [ :each | each isKindOf: aClass ]

I didn't understand why was the select there, but found that testSaveWithSubclasses shows why it's necessary.

Imagine you defined 2 voyages roots: A and B.
If B is contained into A, when you will save A into a MemoryRepository, only A will be saved. The collection holding B will be empty.

The repository dropDatabase after each test run was not very good idea. I have another workaround that works better.

this is because of this:

VOMongoRepositoryResolver>>#execute: aBlock retries: retriesLeft
    aBlock 
    on: NetworkError, BSONError, PrimitiveFailed, ConnectionClosed, VOMongoConnectionError
    do: [ :e | 
        self recoverFromError: e.
        ((self isRetryError: e) and: [ retriesLeft > 0 ])
            ifTrue: [ self execute: aBlock retries: retriesLeft - 1 ]
            ifFalse: [ e resignalAs: VOMongoConnectionError new] ].

and a VOMongoConnectionError is thrown in: VOMongoSessionPool>>#withDatabase:

... then we have an infinite loop :(

a problem in compilation (we need to compile it with static bindings

Tests run ok individually, but they fail when reusing the instance of VOMongoTestResource (a SUnit resource). The #tearDown of such resource executes VOMongoTestResource reset and the #tearDown of the test case just uses VOMongoRepository>>removeAll: to (fast?) reset the resources. Not sure if that's correct, but in any case it shows that: #removeAll: is not removing all elements, which is buggy.

The following code reproduces the bug. It halts around iteration 19 in my computer.

"From resource's tearDown"
VOMongoTestResource reset.

1 to: 30 do: [ :index |

    "From #testSave test case"

    | pilot pilots repository |
    repository := VOMongoTestResource current repository.

    pilot := VOTestPilot new
        name: 'Esteban';
        pet: (VOTestDog new name: 'Doggie').
    repository save: pilot.

    pilots := repository selectAll: VOTestPilot.

    1haltIf: [ pilots size > 1 ].


    "From test case's tearDown"

    repository 
        removeAll: VOTestPet;
        removeAll: VOTestPilot.
].

"From resource's tearDown"
VOMongoTestResource reset.

I think this is somewhat related to #49...

When you have a repository-based cache it means that you cannot manage separate local object copies that may represent object state for a particular view of the repository.

If the cache were managed on a session by session basis, then object lookup and sharing would be guaranteed to be consistent with the db view associated with the session ...

besides, sharing common objects between two sessions can get quite confusing ...

Don't know if this has come up before or not.

If an Error occurs within VOMongoSessionPool>>withDatabase: and VOMongoRepositoryResolver>>execute:retries: is farther down the stack, you end up with an ifinite loop of VOMongoConnectionErrors

VOMongoSessionPool>>withDatabase: eats the original Error turns it into a VOMongoConnectionError which is caught by VOMongoRepositoryResolver>>execute:retries: and if you are out of retries resignals as VOMongoConnectionError and you are off to the races ...

With VOMongoRepositoryResolver>>execute:retries: on the stack the connection error should not be resignalled ... not sure why the error should be turned into a VOMongoConnectionError ...

I suppose with Mongo you are guaranteed that no non-connection-based errors are signalled?

For GemStone, I have my own session pool so I can manage this differently, but any unexpected Errors signalled by the Mongo Smalltalk would result in a similar infinite loop --- not easy to debug

Christophe reported errors using it, because keys seems to be collected before time (We need to confirm and fix o discard)

otherwise you cannot reset :)

If all of the contributors use either GitFileTree or Iceberg in Pharo and tODE for GemStone we can turn the repo into a metadata-less repository and simplify our lives when we hit the point where we try to use pull requests to merge into the same package ...

metadata-less repo is basically required if you expect to have multiple active contrinbutors ...

I have 100k+ objects and access them randomly. This leads to a VOMongoCache of 1000+ entries that are mostly dead (the OID points to nil). The issue is that compaction will occur way too late because the lower limit is too high.

The size of the VOMongoCache adds a measurable latency to the handling of the system. I don't have a workaround but the compactLimit seems to be too high.

VOMongoRepositoryResolver>>#collectionAt: aClass inDatabase: db | collectionName | collectionName := self collectionNameFor: aClass. ^(collections at: collectionName ifAbsentPut: [ db addCollection: collectionName ]) setDatabase: db name: collectionName; yourself.

The returned code is a MongoCollection that will be used concurrently. This means process A will start with database A but if process B uses the same collection and changes it to database B.. process A might make a query against database B (and not A).

This will start failing if both processes make a query at the same time and then one process will "steal" the response from another process. I have worked-around it by adding a "copy" before setting the database. It would be better to have the MongoCollection be cached per process.

I'm in the process of creating a VoyageGemStone implementation and VORepositoryTest has all of the interesting repository tests with no apparent dependencies upon VOMongoTest, so the hierarchy seems to be inverted where VORepositoryTest could be an abstract test that expects subclasses to implement #repository...

I've inverted the hierarchy in my fork and unless there's a reason for the inverted hierarchy I can provide a PR

Current .smalltalk.ston is:

SmalltalkCISpec {
  #loading : [
    SCIMetacelloLoadSpec {
      #baseline : 'Voyage',
      #directory : 'mc',
          #load : [ 'mongo tests' ],      
      #platforms : [ #pharo ]
    }
  ]
}

i.e. it doesn't include either memory nor unqlite.
(Also, #62 introduces Voyage-Mongo-MultipleImageTests package, which I would like to add...)

Proposal: change from tinchodias-1215P test to a VOMongoRepository(tinchodias-1215P:27031/test)

But also refactor the method. BTW, I suspect the super initialize wasn't put on purpose, and I don't think it's necessary to prevent from nils in the variables.

Current:

printOn: aStream 

    super initialize.
    host ifNotNil: [ aStream nextPutAll: ' ', host, ' '].
    databaseName ifNotNil: [ aStream nextPutAll: databaseName ].

Proposed:

printOn: aStream
    super printOn: aStream.

    aStream
        nextPut: $(;
        nextPutAll: host asString;
        nextPut: $:;
        nextPutAll: port asString;
        nextPutAll: ' ';
        print: databaseName asString;
        nextPut: $)

In VOMongoRepository:

host: hostString port: portNumber database: databaseString
    ^self 
        host: hostString 
        port: self defaultPort
        database: databaseString
        username: nil 
        password: nil

because that keyword is mongo-only, as far as I know.
is this correct?

I found a recurrent problem in a project I have... a Seaside project that can access several times same url who ends translated to same lazy element from a collection.
Think is, with this scenario, this fails:

VOLazyProxy>>#doesNotUnderstand: aMessage
    ...
    self becomeForwardKeepingHash: realObject.
    ...

... it fails because of a race condition.
The easiest way I find to "fix" this was to add this:

VOLazyProxy>>#doesNotUnderstand: aMessage
    ...
    VOLazyProxy mutex critical: [ 
        self == realObject
            ifFalse: [ self becomeForwardKeepingHash: realObject ] ].
    ...

like that it works fine, but I wonder if this is the appropriate fix... or the impact on performance...
One possibility to let things as before is to allow the definition of which proxy we use, globally or by collection... but I wonder if this is not too much.

Anyway... opinions?

It would be nice to signal a conflict error when a save/update of an object that has a newer version (in db).
Additionally, repository's api could support something #save:onConflict:.

because I'm resuming to work on the unqlite.org backend, and then we will need to generalise.

So we can do:

Metacello new 
    githubUser: 'pharo-nosql' project: 'voyage' commitish:'1.?' path: 'mc';
    baseline: 'Voyage';
    load: 'mongo'.

etc.

What do you think?