Henrik, Do you think that foo_reload=1 is redundant because SET GLOBAL foo=@@foo will always work in any case?
I think variables like foo_reload are more an abuse of a side effect than foo=@@foo because foo_reload uses a variable as a toggle switch (vs. just a simple on/off switch like the _active variables that I have proposed). I suppose this is ok if we document its semantics, which might be: * foo_reload = 0 when the toggle switch is inactive (i.e. foo is not being reloaded) * foo_reload = 1 while foo is being reloaded (due to SET GLOBAL foo_reload=1) * foo_reload returns to 0 once foo has reloaded, whether successfully or not Still this seems hackish to me. Perhaps we should do the effort of implementing a RELOAD function? This might be a better longterm solution because then we provide reload ability at a higher level so plugin writers don't have to implement special _reload vars. -Daniel Le 29 avr. 2012 à 01:51, Henrik Ingo a écrit : > Daniel, > > I don't like the end user semantics of what you are proposing. It > looks like you are abusing a side effect of something else. > > "something_reload=1" is quite clear what it means. Note that for an > average mysql/drizzle user even the meaning of the @@var_name syntax > might be unclear. > > henrik > > > On Sat, Apr 28, 2012 at 8:56 AM, Daniel Nichter <dan...@percona.com> wrote: >> Anshu, >> >> My reply is quite late, but the conference and Drizzle Day were both >> enlightening. I look forward to next year. >> >> As for updating files, I think there's a simpler, more direct approach: >> >> SET GLOBAL regex_policy_policy=@@regex_policy_policy; >> >> That sets the var to its current value which triggers the plugin's update >> hook and since the new value == the old value, the result is simply to >> reload the file. >> >> This avoid having two code paths: one for SET >> GLOBAL regex_policy_policy="new-file" and another for SET >> GLOBAL regex_policy_reload=1 (which would eventually call the same code as >> the first statement). >> >> Thoughts on this (you and anyone else)? >> >> -Daniel >> >> Le 17 avr. 2012 à 15:09, Anshu Kumar a écrit : >> >> Hi Daniel, >> How was the conference and drizzle day? >> Coming to the point, after brainstorming the possibilities that I have in >> the issue of making regex policy plugin dynamic, I think that the solution I >> proposed in my last mail is best for now. And using that we wont need any >> issue handling with if the policy file was not correct or change in the look >> and feel of some specific plugins. From coding point of view, we can do this >> by adding an option of reload variable which will default be set to false. >> Upon making it true, it will trigger a function which will create a separate >> instance of policy and will call loadfile on it. If the call is a success, >> we will remove all old policies and reload the system with new ones. And if >> the call gives some error message then do nothing. After both the cases, the >> function will change the reload variable to false. This way we wont need any >> separate threads also. >> Waiting for your review comments to start working on this. >> >> On Wed, Apr 4, 2012 at 2:55 PM, Anshu Kumar <ansharyan...@gmail.com> wrote: >>> >>> Hey Daniel, >>> After reading your views, I too think now that auto reloading is not the >>> ideal solution to this scenario. In this solution I have used the idea of >>> thread for reloading the policy files. You put a point that what if admin is >>> using a ext editor which auto saves the changes after some fixed time. In >>> that case incorrect policies may be read. >>> >>> For this issue, instead of using a thread, we can come up with a solution >>> in which the reloading won't be dynamic. So it would be like, if you are an >>> admin and you want to change the policy file, do it. After all changes are >>> done, just change the value of regex_policy_reload to true and policies will >>> then be reloaded. After policies are reloaded, the value of >>> regex_policy_reload will again be set to false from code itself. In this >>> way, it can be ensured that only the correct policies are loaded there. It >>> is true that here too anyone can change the status of regex_policy_reload to >>> true, so either don't let him access the policy file or we will be needing >>> an authorization system for changing global variables too. >>> >>> Brainstorming further for the most optimal solution. Comment and >>> suggestions are welcome. >>> >>> About the name concern, I use Ansh as my nick. Both are correct. You can >>> call me anything you like. However changing my sign to 'Anshu', so that it >>> don't cause any further confusion to anyone. :) >>> >>> On Wed, Apr 4, 2012 at 1:40 PM, Henrik Ingo <henrik.i...@avoinelama.fi> >>> wrote: >>>> >>>> Daniel >>>> >>>> FWIW, I think you might be correct. I originally thought that >>>> auto-reloading would be an easy way to implement a way to re-read the >>>> policy >>>> file. For instance, it could be implemented crudely without any new >>>> configuration options and no new threads needing to launch. However, as >>>> we've gone deeper into the subject it became clear that a good >>>> implementation will need those things, hence the "easy" attribute is no >>>> longer there. I support going back to explore your original idea. >>>> >>>> henrik >>>> >>>> >>>> On Wed, Apr 4, 2012 at 8:12 AM, Daniel Nichter <dan...@percona.com> >>>> wrote: >>>>> >>>>> Anshu, Henrik, Clint, >>>>> >>>>> First: Anshu: I like to use people's names correctly, so I noticed you >>>>> sign your emails "Ansh" but we've been writing "Anshu". Is one or the >>>>> other, or both, correct? >>>>> >>>>> Now on to business. First, thanks for the code. This is a good start >>>>> and I'm happy to see that you've been able to jump into the code with >>>>> apparent ease. >>>>> >>>>> Second, I disagree that the auto-reloading approach is the ideal >>>>> solution. I think we all need to debate the merits of this approach >>>>> further. So let's do that... >>>>> >>>>> In my humble opinion, this approach has the following drawbacks: >>>>> >>>>> 1) It's "gold plating" because in the real world the policy file >>>>> won't/shouldn't change so frequently as to make autp-reloading a valuable >>>>> feature. It's not too much to ask the user to execute one simple command, >>>>> and chances are they will expect to do this. Thinking of Drizzle and >>>>> MySQL, >>>>> no auto-(re)loading features come immediately to my mind, so we won't be >>>>> depriving the user of functionality they're used to. Of course, I'm all >>>>> for >>>>> new types of functionality when it's clear that the functionality will be >>>>> a >>>>> "big win" for users, but given the nature of this issue, I don't think >>>>> auto-reloading is a big win and certainly not worth the extra engineering >>>>> effort. >>>>> >>>>> 2) It's a bit of magic and magic usually leads to problems down the >>>>> road. Sure, it's just one little thread that sleeps, checks, and maybe >>>>> does >>>>> something, but one immediate "problem" comes to mind: testing. Anything >>>>> dependent on time or time-based is inherently more difficult to test. >>>>> It's >>>>> possible, of course, but (related to #1) is it worth the effort? Another >>>>> problem comes to mind: what if the admin is editing the policy file and >>>>> his >>>>> editor auto-saves every N minutes, and auto-reload is on? Drizzle may >>>>> read >>>>> the policy file before the admin intends. Magic can be very difficult to >>>>> wield safely. >>>>> >>>>> 3) It doesn't address the original security concern. Henrik raised the >>>>> problem that anyone could execute the command to reload the policy file, >>>>> but >>>>> anyone could enable the auto-reload, too. The real solution lies in the >>>>> authorization modules. Right now Drizzle has only very basic >>>>> authorization: >>>>> schema and table access (via this plugin nonetheless). What Drizzle needs >>>>> is authorization for setting/changing global variables. I think we can >>>>> make >>>>> still make this dynamic and leave the authorization solution for another >>>>> time/person/project. >>>>> >>>>> 4) It introduces a concept that will either become a norm or become an >>>>> exception. Given the aforementioned points, I wouldn't like to see >>>>> auto-reloading before a norm, and we certainly don't want more exceptions >>>>> because one of our high-level goals is to make the "look and feel" of all >>>>> plugins consistent. So if we do this here, then why not >>>>> --auth-file.auto-reload too? But then we're neck deep in gold plating and >>>>> magic. >>>>> >>>>> 5) It seems contrary to "the Drizzle way". Brian, Stewart, and the >>>>> original core developers have more of a sense of what the Drizzle way is, >>>>> but I'm pretty certain is centers on removing the extraneous and the >>>>> gotchas. Drizzle prides itself it what it doesn't have: no views, no >>>>> triggers, no timezones, etc. I think auto-reloading is extraneous (gold >>>>> plating) and prone to becoming a gotcha. To modify my previous example: >>>>> what if the senior admin enables auto-reloading but forgets to tell the >>>>> junior admin who changes the policy file on Friday and plans to review the >>>>> changes with the senior admin on Monday only for both of them to get a >>>>> call >>>>> during the weekend by the CTO because Sally in HR can't access the >>>>> employees >>>>> database and she's trying to do payroll so now everyone's paychecks will >>>>> be >>>>> late? Gotcha. >>>>> >>>>> So, in conclusion: your code is a good start, but I think we need >>>>> another solution. Other opinions? >>>>> >>>>> -Daniel >>>>> >>>>> >>>>> Le 3 avr. 2012 à 18:03, Anshu Kumar a écrit : >>>>> >>>>> Hi Daniel, >>>>> For making all the plugins dynamic, as the earlier discussion for >>>>> auth_file could not come to a conclusion, I started off with making the >>>>> regex_policy plugin dynamic. Here is the complete process which am >>>>> proposing >>>>> for this plugin >>>>> >>>>> 1. There would be a autoreload variable, default set to false. This >>>>> variable will determine if policy file needs to be reloaded. >>>>> >>>>> 2. Upon setting this variable to true (SET GLOBAL >>>>> regex_policy_autoreload=ON), a new thread will be created which will >>>>> handle >>>>> the reload issues of policy file. >>>>> >>>>> 3. In the thread created, on every one minute, using the stat command to >>>>> find the last modified time and taking the difference from current time, >>>>> it >>>>> will be checked that if the file was modified in last one minute. And if >>>>> it >>>>> was, reload the file. If it wasn't modified, continue to the next poll. >>>>> >>>>> 4. This thing will go on till you again set the variable value to false >>>>> (SET GLOBAL regex_policy_autoreload=OFF) >>>>> >>>>> Scenario would be, if you want to change the policy file and want those >>>>> changes to be reflected, just change the variable to true, do your >>>>> modifications, and the changes will be reflected the next minute in the >>>>> system. >>>>> >>>>> I have implemented this whole thing. Here is the link of the branch. It >>>>> would be great if you can go through it and give your comments. >>>>> https://code.launchpad.net/~ansharyan015/drizzle/dynamic_regex_policy >>>>> >>>>> P.S. Thanks to Henrik for being the continuous help. :) >>>>> Comments and suggestions are welcome. >>>>> >>>>> >>>>> On Mon, Apr 2, 2012 at 11:01 PM, Henrik Ingo <henrik.i...@avoinelama.fi> >>>>> wrote: >>>>>> >>>>>> Anshu: To create a background thread, you need to use class >>>>>> drizzled::plugin::Daemon >>>>>> >>>>>> Look at plugin/json_server/json_server.cc (class JsonServer at the >>>>>> end) for an example. >>>>>> >>>>>> henrik >>>>>> >>>>>> On Mon, Apr 2, 2012 at 4:16 PM, Henrik Ingo <henrik.i...@avoinelama.fi> >>>>>> wrote: >>>>>>> Anshu >>>>>>> >>>>>>> You are right. Creating a thread is more correct. I was just trying >>>>>>> to >>>>>>> avoid it since you originally asked for "low-hanging-fruit" bug. >>>>>>> >>>>>>> Functionally, ignoring the performance hit, you could do this also >>>>>>> from the restrict* methods and then you don't need to create your own >>>>>>> thread. >>>>>>> >>>>>>> henrik >>>>>>> >>>>>>> On Mon, Apr 2, 2012 at 3:45 PM, Anshu Kumar <ansharyan...@gmail.com> >>>>>>> wrote: >>>>>>>> Hey Henrik, >>>>>>>> What I was thinking is we can have a system like this. When >>>>>>>> autoreload >>>>>>>> variable is set to true, we can create a thread which will check >>>>>>>> either by >>>>>>>> inotify or stat() that the corresponding regex policy file is >>>>>>>> changed or >>>>>>>> not. And it of does then only reload the file. This is just like >>>>>>>> creating a >>>>>>>> handler for change in policy file. The performance will be better >>>>>>>> than >>>>>>>> polling for a min and reloading the policy file. >>>>>>>> The implementation is actually the same in this, and the way you >>>>>>>> suggested >>>>>>>> by Policy::restrict methods. So did you want to say that when >>>>>>>> checking for >>>>>>>> regex policy rights (if access is allowed or denied), we can check >>>>>>>> if file >>>>>>>> needs to be refreshed? >>>>>>>> >>>>>>>> On Mon, Apr 2, 2012 at 6:02 PM, Henrik Ingo >>>>>>>> <henrik.i...@avoinelama.fi> >>>>>>>> wrote: >>>>>>>>> >>>>>>>>> Anshu >>>>>>>>> >>>>>>>>> Actually, now that I re-read this, I don't know if it was a smart >>>>>>>>> idea >>>>>>>>> and maybe there needs to be a thread that does the polling, but my >>>>>>>>> original idea was that you could reload the file during an >>>>>>>>> authorization request. So basically when any of the >>>>>>>>> Polixy::restrict... methods are called, you would first check if >>>>>>>>> the >>>>>>>>> file needs to be refreshed and then re-read it (or not). This way >>>>>>>>> you >>>>>>>>> don't need to have a loop or do any polling or such. >>>>>>>>> >>>>>>>>> This approach is not good because it would introduce a performance >>>>>>>>> hit >>>>>>>>> into random requests every N seconds. But you could still do it >>>>>>>>> this >>>>>>>>> way as a proof of concept, then we could work on making it a >>>>>>>>> background thread later. >>>>>>>>> >>>>>>>>> henrik >>>>>>>>> >>>>>>>>> On Mon, Apr 2, 2012 at 2:51 PM, Anshu Kumar >>>>>>>>> <ansharyan...@gmail.com> >>>>>>>>> wrote: >>>>>>>>>> Hey Guys, >>>>>>>>>> I have tried to implement this dynamic thing to regex_policy >>>>>>>>>> plugin. >>>>>>>>>> In reference to my talk with Henrik yesterday, we discussed that >>>>>>>>>> there >>>>>>>>>> could >>>>>>>>>> be a autoreload variable, and when made true it will continuously >>>>>>>>>> poll >>>>>>>>>> for >>>>>>>>>> changes in default regex policy file. And it it is made false, it >>>>>>>>>> will >>>>>>>>>> stop >>>>>>>>>> polling, checking for modifications. Sticking to the discussion, >>>>>>>>>> the >>>>>>>>>> code I >>>>>>>>>> wrote is >>>>>>>>>> >>>>>>>>>> void autoReload_Regex_Policy(Session *, sql_var_t) >>>>>>>>>> { >>>>>>>>>> if(policy->sysvar_autoreload) >>>>>>>>>> { >>>>>>>>>> while(1) >>>>>>>>>> { >>>>>>>>>> if (not policy->loadFile()) >>>>>>>>>> { >>>>>>>>>> errmsg_printf(error::ERROR, _("Could >>>>>>>>>> not >>>>>>>>>> load >>>>>>>>>> regex policy file: %s\n"), >>>>>>>>>> (policy ? >>>>>>>>>> policy->getError().str().c_str() : _("Unknown"))); >>>>>>>>>> return; >>>>>>>>>> } >>>>>>>>>> sleep(60); >>>>>>>>>> if(!policy->sysvar_autoreload) >>>>>>>>>> break; >>>>>>>>>> } >>>>>>>>>> } >>>>>>>>>> else >>>>>>>>>> { >>>>>>>>>> } >>>>>>>>>> } >>>>>>>>>> This function handles the case when you change the value of >>>>>>>>>> autoreload >>>>>>>>>> from >>>>>>>>>> drizzle client. The problem is as the function is using sleep >>>>>>>>>> recursively, >>>>>>>>>> when trying to change autoreload value by "SET GLOBAL", the >>>>>>>>>> client >>>>>>>>>> hangs. >>>>>>>>>> This is obviously due to function recursive structure. Initially, >>>>>>>>>> I >>>>>>>>>> thought >>>>>>>>>> that it would change the variable and then polls. >>>>>>>>>> >>>>>>>>>> Now, coming to the discussion earlier in this mail, even if I >>>>>>>>>> create a >>>>>>>>>> pthread from this function which will check for modification >>>>>>>>>> using >>>>>>>>>> stat() or >>>>>>>>>> inotify(), this function won't exit untill its thread stop >>>>>>>>>> working. And >>>>>>>>>> as >>>>>>>>>> thread will continuously polls for changes, it wont exit. >>>>>>>>>> >>>>>>>>>> Is there any solution for this scenario, except adding the >>>>>>>>>> refresh >>>>>>>>>> command >>>>>>>>>> for refreshing the policy file? >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> On Fri, Mar 30, 2012 at 10:34 PM, Clint Byrum <cl...@fewbar.com> >>>>>>>>>> wrote: >>>>>>>>>>> >>>>>>>>>>> Excerpts from Henrik Ingo's message of Thu Mar 29 21:16:26 -0700 >>>>>>>>>>> 2012: >>>>>>>>>>>> Daniel: >>>>>>>>>>>> >>>>>>>>>>>> Have you thought about authorization for this? I mean we >>>>>>>>>>>> wouldn't >>>>>>>>>>>> want >>>>>>>>>>>> any old logged in user to be able to >>>>>>>>>>>> >>>>>>>>>>>> SET GLOBAL >>>>>>>>>>>> auth_file.users=/home/hingo/igivemyselfrootpowers.users >>>>>>>>>>>> >>>>>>>>>>>> (Making the plugin reload the existing file will be helpful. >>>>>>>>>>>> But it >>>>>>>>>>>> might not be a good idea to allow to change that value.) >>>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> Agreed. I'd like to see plugins like auth_file and regex_policy >>>>>>>>>>> given >>>>>>>>>>> a generic way to "watch" their files. There are a number of ways >>>>>>>>>>> to do >>>>>>>>>>> this, but I don't think each plugin should implement its own >>>>>>>>>>> method. >>>>>>>>>>> >>>>>>>>>>> Thoughts I've had on this: >>>>>>>>>>> >>>>>>>>>>> * A thread which uses either inotify or falls back to polling >>>>>>>>>>> with stat(), and whenever there is a change, calls any >>>>>>>>>>> registered code >>>>>>>>>>> to update that file's effect. >>>>>>>>>>> >>>>>>>>>>> * An admin command like REFRESH '/etc/drizzle/regex.policy' >>>>>>>>>>> which does >>>>>>>>>>> the same thing as the thread without the inotify/polling. >>>>>>>>>>> >>>>>>>>>>> * Cache the stat() call on the file and periodically expire the >>>>>>>>>>> cache >>>>>>>>>>> and refresh the contents if stat() indicates that it has >>>>>>>>>>> changed. >>>> >>>> >>>> >>>> >>>> -- >>>> henrik.i...@avoinelama.fi >>>> +358-40-8211286 skype: henrik.ingo irc: hingo >>>> www.openlife.cc >>>> >>>> My LinkedIn profile: http://www.linkedin.com/profile/view?id=9522559 >>> >>> >>> >>> >>> -- >>> Regards, >>> Anshu >>> >> >> >> >> -- >> Regards, >> Anshu >> >> > > > > -- > henrik.i...@avoinelama.fi > +358-40-8211286 skype: henrik.ingo irc: hingo > www.openlife.cc > > My LinkedIn profile: http://www.linkedin.com/profile/view?id=9522559 _______________________________________________ Mailing list: https://launchpad.net/~drizzle-discuss Post to : drizzle-discuss@lists.launchpad.net Unsubscribe : https://launchpad.net/~drizzle-discuss More help : https://help.launchpad.net/ListHelp
