paul-rogers commented on pull request #2251: URL: https://github.com/apache/drill/pull/2251#issuecomment-885888891
@vdiravka, thanks for the explanation. Let's be careful not to confuse existing implementation with new requirements. Yes, there is a comment that says that the system options are persistent and session options are not. Be careful to read this correctly. *At the time the comment was written*, system options were the only persistent options. System options, by definition, have to be persistent. The comment did not *forbid* session options from being persistent; it simply observes that, at that time, for the reasons we discussed, session options were not persistent. Nor does the comment say that, if you add persistent user-level options, that they have to be system options because only system options are persistent: you are free to add persistence to session options. The gist of your requirement is to let users persist *some* of their session options. We discussed that this may not work as one might think: we have to think through the issues. The point was that concurrency of user options will be a mess if they work like system options: if changes to user/session options are written to persistent storage immediately. This will screw up queries as I persist "all-text-mode" in one session, which breaks a query in another. Point is, user options *must* have semantics different from system options. *Before* we worry about code details, we have to get the semantics right. To repeat: * We need a concurrency guarantee. One choice is "options are propagated to all active sessions with unknown latency, just like system options." Another, more dependable, is "session options are initialized on session startup, after which they do not reflect changes made in other sessions." Whatever it is, the definition has to be stable and explainable. * Some options apply to queries: we've been using "all-text-mode" as an example. Such options will be set frequently. Since these options are per-query, we cannot set the option across all user sessions immediately, as that will break queries. Instead, the user has to *tell us* when they want to persist that setting. Otherwise the setting is per-session (that is, per-query.) Is it a bad thing that the behavior of queries depends on session options? Certainly. That was a poor design choice. But, it is how Drill works, and we have to design new features to reflect this choice. The proposal I outlined is the simplest way to achieve with concurrency semantics we could actually support. Feel free to suggest a better design; but let's discuss that design *before* we reduce it to code. Either way, the design has to reflect our requirements and constraints, not just follow existing code. I strongly recommend splitting this PR into two: one for options, another for plugins. Else, discussion becomes overly complex. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
