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

#2861 - sqlserver input plugin not working with Azure SQL #2864

Closed
wants to merge 1 commit into from
Closed

#2861 - sqlserver input plugin not working with Azure SQL #2864

wants to merge 1 commit into from

Conversation

regevbr
Copy link

@regevbr regevbr commented May 29, 2017

#2861 - sqlserver input plugin not working with Azure SQL

  • Signed CLA.
  • Associated README.md updated.
  • Has appropriate unit tests.

@regevbr regevbr changed the title #2861 #2861 - sqlserver input plugin not working with Azure SQL May 29, 2017
@deluxor
Copy link

deluxor commented May 29, 2017

I will test your PR later, and i'll post some feedback

@regevbr
Copy link
Author

regevbr commented May 29, 2017

Thanks @deluxor

@regevbr
Copy link
Author

regevbr commented May 29, 2017

@deluxor Im done - added a param "disable_parallel" that will cause all queries within a server to run one by one - this is for some reason needed by Azure SQL - probably due to DOS detection.
In addition, added a needed first simple query to sustain the session to the DB (denisenkom/go-mssqldb#217).
In addition, to save resources, each server will have a single connection open to it, instead of 1.
And finally, to resolve the missing sys views, added to the queries options to ignore those views if they don't exist
I replaced the mssql driver because of what @zensqlmonitor said in #2852

@deluxor
Copy link

deluxor commented May 29, 2017

Great, I'll give it a try and back to you soon.

@regevbr
Copy link
Author

regevbr commented May 30, 2017

The new plugin is running in my production env for a few hours now and it works prefect

@danielnelson
Copy link
Contributor

Is this because we are recreating a new connection pool every gather? We have an in progress PR to add this to postgres #2701, would this method work here?

@regevbr
Copy link
Author

regevbr commented Jun 1, 2017

That was not the issue here, but it will make sense to implement the behavior here as well...
There waa a misuse of the connection as it was created per query, which i fixed, but having a global connection will make it better, but I think it belongs to a different pull request

@danielnelson
Copy link
Contributor

Okay, so previously we would create multiple new connections per Gather. With this change we can use a single new connection per Gather?

I don't really want an option to control if queries are run in parallel. I'm not experienced with the go sql library but it would make more sense to me to be able to configure the connection pool size that is used for the life of the plugin.

@regevbr
Copy link
Author

regevbr commented Jun 1, 2017

Yes, single connection per connection string per gathering. Currently every run of the gather method, a new connection is being open, if we want one connection it ia a different request.
As to the parallel queries, it is required to run the queries im sequential order for the DOS mechanism not to get triggered

@danielnelson
Copy link
Contributor

I think adding this option as is leaks too much detail about the implementation. What if we added an option called max_connections? For now we could use a semaphore to limit the connections to this amount but in the future if we add support for a stored connection pool later it would still apply.

@regevbr
Copy link
Author

regevbr commented Jun 1, 2017

Sounds good to me, I will fix the pull request

@danielnelson
Copy link
Contributor

I believe this was addressed in #3618

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.

3 participants