Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade to swift 4.1 and mongodb 3.6, fix vapor tests #3242

Merged
merged 6 commits into from Feb 3, 2018

Conversation

michaelhixson
Copy link
Contributor

See the individual commit messages for details.

This fixes issue TechEmpower#3181.

I think we were running into this issue in the stable vapor release:
https://bugs.swift.org/browse/SR-5800

...which the comments claim is really about this issue:
https://bugs.swift.org/browse/SR-5802

...which was fixed in the main swift source repository:
apple/swift-package-manager#1419

That fix happened pretty recently and doesn't seem to be included in the
latest stable releases (I don't see the PackageManager.swiftdoc file in
those releases), but it does seem to be fixed in the development snapshots
(I do see PackageManager.swiftdoc there).

With the development snapshot, the vapor build progresses much farther.
It eventually runs into an error compiling the vapor framework.  That is
an error in the outdated version of vapor we were using, and it was fixed
later:
vapor/vapor@5a31fc8#diff-4a85e78227146848570c0c42d00d2f17

When I specify 2.4.4 as our version of vapor (the most recent release),
the build is successful and all the tests pass.

I renamed "swift3.sh" to "swift.sh" instead of "swift4.sh" because we
prefer to keep the language version out of these filenames unless we
can't avoid having multiple versions of the same language in the toolset.

It looked like vapor's setup-*.sh files were declaring unnecessary
database dependencies, so I cleaned those up.
The vapor tests rely on a feature that was added in mongodb 3.4, so we
might as well take this opportunity to upgrade mongodb to the latest
release.
They fail verification locally because they take too long to produce a
response.
It's showing the wrong version of Vapor, at least.  Also, it's not clear
to me if "Package.pins" is still used in this version of swift or if it
was replaced by "Package.resolved".
Some frameworks query for worlds by _id, and others by id.  It looks like
the ones that queried by id were being penalized heavily because we had
no index on that field.  In local testing, the RPS of nodejs-mongodb in
the db test went up by 8x (!) after I added the index.

Use createIndex instead of ensureIndex because, according to mongodb's
documentation, ensureIndex is:

  Deprecated since version 3.0.0: db.collection.ensureIndex() is now an
  alias for db.collection.createIndex().

Remove comment with a link to mongodb optimization because it doesn't
seem to say anything related to what we're doing in this file.
The restexpress build on travis is failing with an exception like this:

  Exception in thread "main" com.mongodb.MongoException: Index with name: id_1 already exists with different options
    at com.mongodb.CommandResult.getException(CommandResult.java:100)
    at com.mongodb.CommandResult.throwOnError(CommandResult.java:134)
    at com.mongodb.DBTCPConnector._checkWriteError(DBTCPConnector.java:142)
    at com.mongodb.DBTCPConnector.say(DBTCPConnector.java:183)
    at com.mongodb.DBTCPConnector.say(DBTCPConnector.java:155)
    at com.mongodb.DBApiLayer$MyCollection.insert(DBApiLayer.java:270)
    at com.mongodb.DBApiLayer$MyCollection.createIndex(DBApiLayer.java:365)
    at com.mongodb.DBCollection.createIndex(DBCollection.java:484)
    at com.mongodb.DBCollection.ensureIndex(DBCollection.java:560)
    at com.github.jmkgreen.morphia.DatastoreImpl.ensureIndex(DatastoreImpl.java:431)
    at com.github.jmkgreen.morphia.DatastoreImpl.ensureIndexes(DatastoreImpl.java:536)
    at com.github.jmkgreen.morphia.DatastoreImpl.ensureIndexes(DatastoreImpl.java:495)
    at com.github.jmkgreen.morphia.DatastoreImpl.ensureIndexes(DatastoreImpl.java:584)
    at com.github.jmkgreen.morphia.DatastoreImpl.ensureIndexes(DatastoreImpl.java:573)
    at com.strategicgains.repoexpress.mongodb.MongodbRepository.initialize(MongodbRepository.java:106)
    at com.strategicgains.repoexpress.mongodb.MongodbRepository.<init>(MongodbRepository.java:67)
    at hello.controller.persistence.WorldsMongodbRepository.<init>(WorldsMongodbRepository.java:14)
    at hello.config.Configuration.initialize(Configuration.java:53)
    at hello.config.Configuration.fillValues(Configuration.java:45)
    at com.strategicgains.restexpress.util.Environment.load(Environment.java:61)
    at com.strategicgains.restexpress.util.Environment.from(Environment.java:50)
    at com.strategicgains.restexpress.util.Environment.fromDefault(Environment.java:40)
    at hello.Main.loadEnvironment(Main.java:49)
    at hello.Main.main(Main.java:18)

Individual frameworks should not be attempting to modify the indexes in
the database.  Hopefully that's all this @indexed annotation was doing.
@NateBrady23 NateBrady23 mentioned this pull request Feb 3, 2018
11 tasks
@NateBrady23 NateBrady23 merged commit e57a56d into TechEmpower:master Feb 3, 2018
zloster pushed a commit to zloster/FrameworkBenchmarks that referenced this pull request Feb 7, 2018
* Upgrade to swift 4, repair vapor build

This fixes issue TechEmpower#3181.

I think we were running into this issue in the stable vapor release:
https://bugs.swift.org/browse/SR-5800

...which the comments claim is really about this issue:
https://bugs.swift.org/browse/SR-5802

...which was fixed in the main swift source repository:
apple/swift-package-manager#1419

That fix happened pretty recently and doesn't seem to be included in the
latest stable releases (I don't see the PackageManager.swiftdoc file in
those releases), but it does seem to be fixed in the development snapshots
(I do see PackageManager.swiftdoc there).

With the development snapshot, the vapor build progresses much farther.
It eventually runs into an error compiling the vapor framework.  That is
an error in the outdated version of vapor we were using, and it was fixed
later:
vapor/vapor@5a31fc8#diff-4a85e78227146848570c0c42d00d2f17

When I specify 2.4.4 as our version of vapor (the most recent release),
the build is successful and all the tests pass.

I renamed "swift3.sh" to "swift.sh" instead of "swift4.sh" because we
prefer to keep the language version out of these filenames unless we
can't avoid having multiple versions of the same language in the toolset.

It looked like vapor's setup-*.sh files were declaring unnecessary
database dependencies, so I cleaned those up.

* Upgrade mongdb from 3.2 to 3.6

The vapor tests rely on a feature that was added in mongodb 3.4, so we
might as well take this opportunity to upgrade mongodb to the latest
release.

* Disable vapor+mongodb multi-query and updates endpoints

They fail verification locally because they take too long to produce a
response.

* Remove vapor's Package.pins file, which is out of date and maybe unused?

It's showing the wrong version of Vapor, at least.  Also, it's not clear
to me if "Package.pins" is still used in this version of swift or if it
was replaced by "Package.resolved".

* Add index for world.id in mongodb

Some frameworks query for worlds by _id, and others by id.  It looks like
the ones that queried by id were being penalized heavily because we had
no index on that field.  In local testing, the RPS of nodejs-mongodb in
the db test went up by 8x (!) after I added the index.

Use createIndex instead of ensureIndex because, according to mongodb's
documentation, ensureIndex is:

  Deprecated since version 3.0.0: db.collection.ensureIndex() is now an
  alias for db.collection.createIndex().

Remove comment with a link to mongodb optimization because it doesn't
seem to say anything related to what we're doing in this file.

* Try to prevent restexpress from creating its own index in mongodb

The restexpress build on travis is failing with an exception like this:

  Exception in thread "main" com.mongodb.MongoException: Index with name: id_1 already exists with different options
    at com.mongodb.CommandResult.getException(CommandResult.java:100)
    at com.mongodb.CommandResult.throwOnError(CommandResult.java:134)
    at com.mongodb.DBTCPConnector._checkWriteError(DBTCPConnector.java:142)
    at com.mongodb.DBTCPConnector.say(DBTCPConnector.java:183)
    at com.mongodb.DBTCPConnector.say(DBTCPConnector.java:155)
    at com.mongodb.DBApiLayer$MyCollection.insert(DBApiLayer.java:270)
    at com.mongodb.DBApiLayer$MyCollection.createIndex(DBApiLayer.java:365)
    at com.mongodb.DBCollection.createIndex(DBCollection.java:484)
    at com.mongodb.DBCollection.ensureIndex(DBCollection.java:560)
    at com.github.jmkgreen.morphia.DatastoreImpl.ensureIndex(DatastoreImpl.java:431)
    at com.github.jmkgreen.morphia.DatastoreImpl.ensureIndexes(DatastoreImpl.java:536)
    at com.github.jmkgreen.morphia.DatastoreImpl.ensureIndexes(DatastoreImpl.java:495)
    at com.github.jmkgreen.morphia.DatastoreImpl.ensureIndexes(DatastoreImpl.java:584)
    at com.github.jmkgreen.morphia.DatastoreImpl.ensureIndexes(DatastoreImpl.java:573)
    at com.strategicgains.repoexpress.mongodb.MongodbRepository.initialize(MongodbRepository.java:106)
    at com.strategicgains.repoexpress.mongodb.MongodbRepository.<init>(MongodbRepository.java:67)
    at hello.controller.persistence.WorldsMongodbRepository.<init>(WorldsMongodbRepository.java:14)
    at hello.config.Configuration.initialize(Configuration.java:53)
    at hello.config.Configuration.fillValues(Configuration.java:45)
    at com.strategicgains.restexpress.util.Environment.load(Environment.java:61)
    at com.strategicgains.restexpress.util.Environment.from(Environment.java:50)
    at com.strategicgains.restexpress.util.Environment.fromDefault(Environment.java:40)
    at hello.Main.loadEnvironment(Main.java:49)
    at hello.Main.main(Main.java:18)

Individual frameworks should not be attempting to modify the indexes in
the database.  Hopefully that's all this @indexed annotation was doing.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants