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

Fix/init data race #40

Open
wants to merge 3 commits into
base: public
Choose a base branch
from
Open

Conversation

hwchen
Copy link

@hwchen hwchen commented Oct 22, 2018

@jazzido @jspeis just wanted to start the conversation about patching the race condition on connection init.

As far as I can tell, this patch fixes NilClass errors that were occurring when there were near-simultaneous requests to MondrianRest.

Previously, constructing the connection and connecting occurred on the first api request. Since connecting took some time, there would be a moment when @@olap was no longer nil, but the new mondrian connection was not yet ready for use (resulting in, we think, the NilClass errors).

With this patch, now those initialization steps are done in the config.ru.

mondrian_conn = Mondrian::OLAP::Connection.new(PARAMS)
mondrian_conn.connect
Mondrian::REST::APIHelpers.class_variable_set('@@olap', mondrian_conn)

use Rack::Config do |env|
  env['mondrian-olap.conn'] = mondrian_conn
end

There's probably a better way to do this, so please let us know what you think @jazzido . This would be a pretty big breaking change if it had to be done in config.ru

@hwchen
Copy link
Author

hwchen commented Oct 22, 2018

Oops, doesn't fix. Increasing the threads and the NilClass behavior returns.

@hwchen hwchen closed this Oct 22, 2018
@hwchen hwchen reopened this Oct 23, 2018
@hwchen
Copy link
Author

hwchen commented Oct 23, 2018

Ok, so some modifications and it looks like nilClass has not returned.

Changing the reference from olap to @@olap in get_cubes_or_404 appeared to get rid of the last NilClass.

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.

1 participant