Dynamic parameters and repeated calls to the server #689

Open
FriedrichWeinmann opened this Issue Feb 9, 2017 · 4 comments

Projects

None yet

2 participants

@FriedrichWeinmann
Contributor

Is this a feature OR bug:

Probably a bug, definitely not a feature

Issue

The dynamic parameter code will be run repeatedly while adding parameters, browsing options, etc. This means, it will try to establish connections with the Server multiple times - even before credentials are specified, necessary or not.

Potential Consequences

  • Reduced Performance, especially when connecting to remote servers
  • Lot's of failed login attempts when working on a machine for a while, which may make the next security audit ... awkward.

Proposed solution

  • Keep a cache in memory over whether the server can handle login without credentials. Give each such info a ttl (In case configurations change). This will dramatically cut down on the bad connection attempts.
  • Keep a short-term cache on the results of dynamic parameter (on a per function base). This will keep from repeatedly querying for the same results

The first option could be easily implemented in Connect-SqlServer.
The second option would require a modification on a per parameter basis.

@FriedrichWeinmann
Contributor

Now, should this be addressed?
I'll be happy to implement the first solution, the second one will require a lot more work though.

@ctrlbold
Member

Really great points and caching sounds awesome. How does that work? Will it be needed when we move to TE++? Thats the dynamic param replacement written by Jason Shirk and adopted in v5 (but we gotta be backwards compat for v3). If you need me to look it up, let me know. I can, but it'll take a couple days. I did some work on it in the dynamicparams branch.

@FriedrichWeinmann
Contributor

No worries, just checked it out myself:
Good news:

  • TEPP won't flood requests. So no it will not be necessary.
    Caching negatives could still improve performance, but odds are the overhead would outweigh that in most scenarios

It would work by keeping a list of Servers that plain reject your windows authentication. The first time you try and fail, you note that down in a script level variable (servername and timestamp). You then configure a timeout and within that time, whenever you try to access the server without specifying credentials, Connect-SqlServer instantly errors out, rather than even try.
We also specify a very short timeout we attach to a function when it successfully retrieves the results (for less dynamic content, such as list of available databases). If the function does the same request within that timespan, we give it the previous results, rather than request them again.

So basically, all it needs is a hashtable on a script-scope (that is: Only visible from within the module) to store either connection errors or results, together with a timeout.
Some performance improvements would be possible in combination with a C# library, but that's not really essential, if we move to TEPP for 1.0.

Are we going to enforce TEPP dependency or will that be an optional integration with the clear advice to get it?

@ctrlbold
Member

I will process your response after some yerba tomorrow ;) But for the last part, similarly to Invoke-Sqlcmd, I'd just auto copy the necessary parts (and only the necessary parts) for each release (to add to publish.ps1)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment