Hey Daniel, What do you exactly mean by a 'RELOAD' function? If that is a generic function which can be used for every of the plugin which needs to be reloaded, then of course it is the best solution.
Additionally, apart from using a reload variable as I earlier proposed, the other option of using just a single variable, for eg, regex_policy_policy may not be that obvious for the user. Elaborating on the point, what is a person changes the regex_policy_policy to something else and then he again makes some changes in the new file. Setting the value of regex_policy_policy is not a very obvious thing for the user (the essence is of changing the policy file, not reloading it). regex_policy_reload is more clear in terms from this. Ofcourse we will have to document in a clear way. On Sun, Apr 29, 2012 at 8:04 PM, Daniel Nichter <dan...@percona.com> wrote: > 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 > > -- Regards, Anshu
_______________________________________________ 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
