Skip to content

Conversation

@Kami
Copy link
Member

@Kami Kami commented Feb 9, 2017

This pull request updates the code to ignore the errors about which we can't do anything right now.

Correct solution we should eventually do is to re-creating offending indexes. The problem with re-creation is that the operation is blocking and depending on the dataset size, it could take a very long time.

Kami added 2 commits February 9, 2017 17:04
Right now we can't do anything about those errors, proper solution would
involve, re-creating the index, but this could be very expensive and
blocking.
@Kami
Copy link
Member Author

Kami commented Feb 9, 2017

To give some context - MongoDB 3.4 is much stricter with index validation and uid field index creation would fail on bootstrap when running on existing installation because index with the same name, but different fields already exist (types index option was removed from MongoDB 3.4).

This would cause a failure, but the error is not actually fatal - old / existing index still works just fine.

As mentioned above, real long-term solution is to re-work the ensure_indexes code to re-create offending indexes, but we will still need to be careful we don't mess with indexes in such manner unless is really needed because it re-creation would be very slow and expensive.

@codecov-io
Copy link

codecov-io commented Feb 9, 2017

Codecov Report

Merging #3202 into master will increase coverage by <.01%.
The diff coverage is 70.59%.

@@            Coverage Diff             @@
##           master    #3202      +/-   ##
==========================================
+ Coverage   77.83%   77.84%   +<.01%     
==========================================
  Files         433      433              
  Lines       22398    22415      +17     
==========================================
+ Hits        17433    17447      +14     
- Misses       4965     4968       +3
Impacted Files Coverage Δ
st2common/st2common/models/db/init.py 94.15% <70.59%> (-1.07%)
st2api/st2api/controllers/v1/policies.py 71.76% <ø> (-6.11%)
st2common/st2common/bootstrap/sensorsregistrar.py 80.58% <ø> (-4.85%)
st2api/st2api/controllers/v1/packs.py 90.74% <ø> (-2.47%)
st2common/st2common/rbac/utils.py 92.96% <ø> (-1.41%)
st2common/st2common/bootstrap/triggersregistrar.py 83.33% <ø> (-1.19%)
st2common/st2common/transport/consumers.py 86.32% <ø> (-1.05%)
st2api/st2api/controllers/v1/keyvalue.py 84.42% <ø> (-0.65%)
st2common/st2common/rbac/resolvers.py 88.62% <ø> (-0.53%)
st2api/st2api/controllers/resource.py 87.19% <ø> (+0.41%)
... and 11 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ce0eb95...ed71024. Read the comment docs.

background: true
# We sleep a bit to wait for the background process to start and script to
# finish
- case $CIRCLE_NODE_INDEX in [1,2]) sleep 10 ;; esac
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't we just make it a part of the previous line?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sadly no because it runs in a background (background: true) and that's why we need it to be a separate step.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about the next one? =)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It can but I prefer separate lines to make it more readable (flow it's more sequential).

I already hate all the case lines which make it hard to read and follow so I plan to look into circleci-matrix or similar solution in the future so we can at the very least keep circle.yml more sane.

indexes.

Sadly there is no way around it but to drop the offending index and
re-create it

See http://docs.mongoengine.org/upgrade.html#inheritance
@Kami
Copy link
Member Author

Kami commented Feb 10, 2017

It looks like there is sadly no way around it but to drop and re-create the offending index - ebcf8cb.

Luckily this only affects the "uid" index.

See https://docs.mongoengine.org/upgrade.html#inheritance for details on the types stuff.

As you can see in the diff, I only scoped it to the uid index to be on the safe side although something like this would also be useful in other contexts in the future.

Copy link
Contributor

@m4dcoder m4dcoder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM once the packages and CIs pass. I was wondering about this index error I was seeing on my dev system. I had to clean the mongoDB but obviously not good for upgrades. Thanks for the fix.

@Kami
Copy link
Member Author

Kami commented Feb 13, 2017

It looks like the e2e issue was the same issue as one here - #3211 (comment)

Syncing with latest master should solve it.

@Kami Kami merged commit be6acb4 into master Feb 13, 2017
@Kami Kami deleted the ignore_index_already_exists_errors branch February 13, 2017 10:07
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.

5 participants