Hello,
A desire had been expressed to get rid of referint plugin's logfile: https://pagure.io/389-ds-base/issue/49202
It finally turns out that this file is used for other purposes than real logging.
The referint plugin currently works like that; When the delay update is set to be more than 0 a new thread is created executing referential integrity code every x seconds (set by delay update). When a delete or modrdn operation happens, the plugin will write that down to its logfile. So, every x seconds the plugin will check the logfile, see what happened and apply the changes. Finally, it deletes the file, thus clearing the state for the next time it reads from it.
After discussing this with William he suggested it's better to replace the file with a queue, since the file involves excess fsync / sync, and has all kinds of potential state/race issues. Using a queue will be much faster as well.
William went even further and suggested that we could get rid of the async referint update completely. This probably wouldn't happen soon though, since likely customers are using it. For now we could provide a warning such as "we recommend you set delay to 0".
Finally, the referint-logchanges attribute does absolutely nothing. It seems to be completely ignored by the plugin, so we could remove this as well.
I'll start working on these changes soon.
Any thoughts or objections on the above would be welcome.
Thanks, Ilias
On 07/06/2017 01:07 PM, Ilias Stamatis wrote:
Hello,
A desire had been expressed to get rid of referint plugin's logfile: https://pagure.io/389-ds-base/issue/49202
It finally turns out that this file is used for other purposes than real logging.
The referint plugin currently works like that; When the delay update is set to be more than 0 a new thread is created executing referential integrity code every x seconds (set by delay update). When a delete or modrdn operation happens, the plugin will write that down to its logfile. So, every x seconds the plugin will check the logfile, see what happened and apply the changes. Finally, it deletes the file, thus clearing the state for the next time it reads from it.
After discussing this with William he suggested it's better to replace the file with a queue, since the fileinvolves excess fsync / sync, and has all kinds of potential state/race issues. Using a queue will be much faster as well.
William went even further and suggested that we could get rid of the async referint update completely. This probably wouldn't happen soon though, since likely customers are using it. For now we could provide a warning such as "we recommend you set delay to 0".
Finally, the referint-logchanges attribute does absolutely nothing. It seems to be completely ignored by the plugin, so we could remove this as well.
I'll start working on these changes soon.
Any thoughts or objections on the above would be welcome.
The only problem with going to a queue is if the server goes down unexpectedly. In such a case those RI updates would be lost.
This also brings up a different point... the RI plugin is a backend txn plugin. If we write changes to a log, and those changes end up failing for some reason, then there is no way to rollback the original transaction --> breaking the backend txn plugin model.
Perhaps the log/delay should just be removed? Or ignore the log/delay settings if the plugin is set as a backend txn plugin?
Thanks, Ilias
389-devel mailing list -- 389-devel@lists.fedoraproject.org To unsubscribe send an email to 389-devel-leave@lists.fedoraproject.org
On Thu, 2017-07-06 at 14:33 -0400, Mark Reynolds wrote:
On 07/06/2017 01:07 PM, Ilias Stamatis wrote:
Hello,
A desire had been expressed to get rid of referint plugin's logfile: https://pagure.io/389-ds-base/issue/49202
It finally turns out that this file is used for other purposes than real logging.
The referint plugin currently works like that; When the delay update is set to be more than 0 a new thread is created executing referential integrity code every x seconds (set by delay update). When a delete or modrdn operation happens, the plugin will write that down to its logfile. So, every x seconds the plugin will check the logfile, see what happened and apply the changes. Finally, it deletes the file, thus clearing the state for the next time it reads from it.
After discussing this with William he suggested it's better to replace the file with a queue, since the fileinvolves excess fsync / sync, and has all kinds of potential state/race issues. Using a queue will be much faster as well.
William went even further and suggested that we could get rid of the async referint update completely. This probably wouldn't happen soon though, since likely customers are using it. For now we could provide a warning such as "we recommend you set delay to 0".
Finally, the referint-logchanges attribute does absolutely nothing. It seems to be completely ignored by the plugin, so we could remove this as well.
I'll start working on these changes soon.
Any thoughts or objections on the above would be welcome.
The only problem with going to a queue is if the server goes down unexpectedly. In such a case those RI updates would be lost.
We already have this issue because there is a delay between the change to the object and the log being sync() to disk. So we can already lose changes here. TBH the only fix is ot remove the async model. I actually question why we still need async/delay processing of the refint plugin ...
This also brings up a different point... the RI plugin is a backend txn plugin. If we write changes to a log, and those changes end up failing for some reason, then there is no way to rollback the original transaction --> breaking the backend txn plugin model.
Perhaps the log/delay should just be removed? Or ignore the log/delay settings if the plugin is set as a backend txn plugin?
Completely agree. Because of the delay, if we roll back the txn we still do the refint check.
I would be fully in support of removing the delay option and going betxn for the plugin only. This delay behaviour is the reason we advise you only run refint on one master in a topology, where if we remove this and go betxn, we can run on all masters correctly. I think we would need to make the plugin ignore replicated ops then too.
My only concern would be what version to have this change land in - as much as I'm excited to make the change we should be careful.
Perhaps we remove the delay processing, and have the "delay" process flag act as a switch to check incoming repl ops? Because today if you have delay > 0, you likely have refint on one master, so we need to refint incoming repl ops. If you have delay 0, you ignore repl ops because you assume all masters have refint?
No matter what, it's not a smooth upgrade process here, but I think long term it's nicer to just have it on "all masters".
On 07/06/2017 08:13 PM, William Brown wrote:
On Thu, 2017-07-06 at 14:33 -0400, Mark Reynolds wrote:
On 07/06/2017 01:07 PM, Ilias Stamatis wrote:
Hello,
A desire had been expressed to get rid of referint plugin's logfile: https://pagure.io/389-ds-base/issue/49202
It finally turns out that this file is used for other purposes than real logging.
The referint plugin currently works like that; When the delay update is set to be more than 0 a new thread is created executing referential integrity code every x seconds (set by delay update). When a delete or modrdn operation happens, the plugin will write that down to its logfile. So, every x seconds the plugin will check the logfile, see what happened and apply the changes. Finally, it deletes the file, thus clearing the state for the next time it reads from it.
After discussing this with William he suggested it's better to replace the file with a queue, since the fileinvolves excess fsync / sync, and has all kinds of potential state/race issues. Using a queue will be much faster as well.
William went even further and suggested that we could get rid of the async referint update completely. This probably wouldn't happen soon though, since likely customers are using it. For now we could provide a warning such as "we recommend you set delay to 0".
Finally, the referint-logchanges attribute does absolutely nothing. It seems to be completely ignored by the plugin, so we could remove this as well.
I'll start working on these changes soon.
Any thoughts or objections on the above would be welcome.
The only problem with going to a queue is if the server goes down unexpectedly. In such a case those RI updates would be lost.
We already have this issue because there is a delay between the change to the object and the log being sync() to disk. So we can already lose changes here. TBH the only fix is ot remove the async model. I actually question why we still need async/delay processing of the refint plugin ...
Historically speaking, a long time ago, we used to see high CPU when the RI plugin was engaged. Setting the delay to 1 second, and allowing the log thread to do the work, improved performance. Of course this is now obsolete with the betxn plugin model and other improvements, but I wanted to share why the feature even existed.
This also brings up a different point... the RI plugin is a backend txn plugin. If we write changes to a log, and those changes end up failing for some reason, then there is no way to rollback the original transaction --> breaking the backend txn plugin model.
Perhaps the log/delay should just be removed? Or ignore the log/delay settings if the plugin is set as a backend txn plugin?
Completely agree. Because of the delay, if we roll back the txn we still do the refint check.
I would be fully in support of removing the delay option and going betxn for the plugin only. This delay behaviour is the reason we advise you only run refint on one master in a topology, where if we remove this and go betxn, we can run on all masters correctly. I think we would need to make the plugin ignore replicated ops then too.
My only concern would be what version to have this change land in - as much as I'm excited to make the change we should be careful.
Perhaps we remove the delay processing, and have the "delay" process flag act as a switch to check incoming repl ops? Because today if you have delay > 0, you likely have refint on one master, so we need to refint incoming repl ops. If you have delay 0, you ignore repl ops because you assume all masters have refint?
No matter what, it's not a smooth upgrade process here, but I think long term it's nicer to just have it on "all masters".
389-devel mailing list -- 389-devel@lists.fedoraproject.org To unsubscribe send an email to 389-devel-leave@lists.fedoraproject.org
Any thoughts or objections on the above would be welcome.
The only problem with going to a queue is if the server goes down unexpectedly. In such a case those RI updates would be lost.
We already have this issue because there is a delay between the change to the object and the log being sync() to disk. So we can already lose changes here. TBH the only fix is ot remove the async model. I actually question why we still need async/delay processing of the refint plugin ...
Historically speaking, a long time ago, we used to see high CPU when the RI plugin was engaged. Setting the delay to 1 second, and allowing the log thread to do the work, improved performance. Of course this is now obsolete with the betxn plugin model and other improvements, but I wanted to share why the feature even existed.
I guess that would be related to internal op searches / lack of indexing. These days it's not as big of an issue.
So, lets open a ticket to remove delayed processing mode then?
On 07/07/2017 07:10 AM, William Brown wrote:
Any thoughts or objections on the above would be welcome.
The only problem with going to a queue is if the server goes down unexpectedly. In such a case those RI updates would be lost.
We already have this issue because there is a delay between the change to the object and the log being sync() to disk. So we can already lose changes here. TBH the only fix is ot remove the async model. I actually question why we still need async/delay processing of the refint plugin ...
Historically speaking, a long time ago, we used to see high CPU when the RI plugin was engaged. Setting the delay to 1 second, and allowing the log thread to do the work, improved performance. Of course this is now obsolete with the betxn plugin model and other improvements, but I wanted to share why the feature even existed.
I guess that would be related to internal op searches / lack of indexing. These days it's not as big of an issue.
boldly said. How do you know, did you verify it ? we have seen many customer issues with refereint which were resolved by making it async, just removing this option without proof of a better solution is no good. I also am not sure if we need to tie anything into the betxn. There are operations, which, in my opinion, can be delayed, even redone by fixup tasks, so it is not necessary to have it in one txn, and if the option is there to delay it if you want, we should not take it away
So, lets open a ticket to remove delayed processing mode then?
you can, but I will oppose to implement it :-)
389-devel mailing list -- 389-devel@lists.fedoraproject.org To unsubscribe send an email to 389-devel-leave@lists.fedoraproject.org
On 07/07/2017 10:44 AM, Ludwig Krispenz wrote:
On 07/07/2017 07:10 AM, William Brown wrote:
Any thoughts or objections on the above would be welcome.
The only problem with going to a queue is if the server goes down unexpectedly. In such a case those RI updates would be lost.
We already have this issue because there is a delay between the change to the object and the log being sync() to disk. So we can already lose changes here. TBH the only fix is ot remove the async model. I actually question why we still need async/delay processing of the refint plugin ...
Historically speaking, a long time ago, we used to see high CPU when the RI plugin was engaged. Setting the delay to 1 second, and allowing the log thread to do the work, improved performance. Of course this is now obsolete with the betxn plugin model and other improvements, but I wanted to share why the feature even existed.
I guess that would be related to internal op searches / lack of indexing. These days it's not as big of an issue.
boldly said. How do you know, did you verify it ? we have seen many customer issues with refereint which were resolved by making it async, just removing this option without proof of a better solution is no good. I also am not sure if we need to tie anything into the betxn. There are operations, which, in my opinion, can be delayed, even redone by fixup tasks, so it is not necessary to have it in one txn, and if the option is there to delay it if you want, we should not take it away
So, lets open a ticket to remove delayed processing mode then?
you can, but I will oppose to implement it :-)
I would even go and suggest to implement the delay feature for memberof, like a continuous fixup task.
389-devel mailing list --389-devel@lists.fedoraproject.org To unsubscribe send an email to389-devel-leave@lists.fedoraproject.org
-- Red Hat GmbH,http://www.de.redhat.com/, Registered seat: Grasbrunn, Commercial register: Amtsgericht Muenchen, HRB 153243, Managing Directors: Charles Cachera, Michael Cunningham, Michael O'Neill, Eric Shander
389-devel mailing list -- 389-devel@lists.fedoraproject.org To unsubscribe send an email to 389-devel-leave@lists.fedoraproject.org
On Fri, 2017-07-07 at 11:44 +0200, Ludwig Krispenz wrote:
On 07/07/2017 10:44 AM, Ludwig Krispenz wrote:
On 07/07/2017 07:10 AM, William Brown wrote:
> Any thoughts or objections on the above would be welcome. The only problem with going to a queue is if the server goes down unexpectedly. In such a case those RI updates would be lost.
We already have this issue because there is a delay between the change to the object and the log being sync() to disk. So we can already lose changes here. TBH the only fix is ot remove the async model. I actually question why we still need async/delay processing of the refint plugin ...
Historically speaking, a long time ago, we used to see high CPU when the RI plugin was engaged. Setting the delay to 1 second, and allowing the log thread to do the work, improved performance. Of course this is now obsolete with the betxn plugin model and other improvements, but I wanted to share why the feature even existed.
I guess that would be related to internal op searches / lack of indexing. These days it's not as big of an issue.
boldly said. How do you know, did you verify it ? we have seen many customer issues with refereint which were resolved by making it async, just removing this option without proof of a better solution is no good. I also am not sure if we need to tie anything into the betxn. There are operations, which, in my opinion, can be delayed, even redone by fixup tasks, so it is not necessary to have it in one txn, and if the option is there to delay it if you want, we should not take it away
So, lets open a ticket to remove delayed processing mode then?
you can, but I will oppose to implement it :-)
I would even go and suggest to implement the delay feature for memberof, like a continuous fixup task.
I disagree: Right now a delayed / async mode causes us issue because you have a seperation between "the change" then the database reflecting that. Be it through memberof or refint. We have lost the atomic nature of the DB as a result.
Memberof especially so because SSSD uses this for membership. If we implemented this we could cause undue delays to access controls be it addition or removal while a consumer waits for the processing to occur.
Additionally, the async modes *force* the plugin to run on a single master, rather than all masters lest we cause many replication conflicts.
For the sake of replication master consistency being able to run the same configuration on all masters is extremely important.
Finally, any delays in refint/memberof processing:
* We have a better MO algorithm there, it's just not been implemented yet. * Perhaps refint is missing indexes, or needs an algorithmic change of it's own.
I think that it's a better investment of time to *fix* our problems, rather than hide and delay them behind async processing tasks (which arguably, will cause other random delays by being batched, and holding the write lock for a longer time period than a single write).
So, I will open this ticket, and I think that given your concerns about this Ludwig, we'll need to keep in mind that refint processing performance is a key aspect of this change, and that we will need to perform some significant load testing to guarantee we will not cause a regression in this space.
On 07/10/2017 12:13 AM, William Brown wrote:
On Fri, 2017-07-07 at 11:44 +0200, Ludwig Krispenz wrote:
On 07/07/2017 10:44 AM, Ludwig Krispenz wrote:
On 07/07/2017 07:10 AM, William Brown wrote:
>> Any thoughts or objections on the above would be welcome. > The only problem with going to a queue is if the server goes down > unexpectedly. In such a case those RI updates would be lost. We already have this issue because there is a delay between the change to the object and the log being sync() to disk. So we can already lose changes here. TBH the only fix is ot remove the async model. I actually question why we still need async/delay processing of the refint plugin ...
Historically speaking, a long time ago, we used to see high CPU when the RI plugin was engaged. Setting the delay to 1 second, and allowing the log thread to do the work, improved performance. Of course this is now obsolete with the betxn plugin model and other improvements, but I wanted to share why the feature even existed.
I guess that would be related to internal op searches / lack of indexing. These days it's not as big of an issue.
boldly said. How do you know, did you verify it ? we have seen many customer issues with refereint which were resolved by making it async, just removing this option without proof of a better solution is no good. I also am not sure if we need to tie anything into the betxn. There are operations, which, in my opinion, can be delayed, even redone by fixup tasks, so it is not necessary to have it in one txn, and if the option is there to delay it if you want, we should not take it away
So, lets open a ticket to remove delayed processing mode then?
you can, but I will oppose to implement it :-)
I would even go and suggest to implement the delay feature for memberof, like a continuous fixup task.
I disagree: Right now a delayed / async mode causes us issue because you have a seperation between "the change" then the database reflecting that. Be it through memberof or refint. We have lost the atomic nature of the DB as a result.
yes and no. A client operation does not know which plugins are enabled, if they are delayed, betxn or postop plugins, or if they are temporarily disabled. We still have the ACID property for the client. if we set the delay the main operation and the secondary operation eg meberof or refint are no longer atomic. But this is deliberate decision, you also cannot prevent an admin to disable memberof for some time and later run fixup, in that period group changes and memberof also are not atomic.
Memberof especially so because SSSD uses this for membership. If we implemented this we could cause undue delays to access controls be it addition or removal while a consumer waits for the processing to occur.
who said that we have to use a delay in this deployment ?
Additionally, the async modes *force* the plugin to run on a single master, rather than all masters lest we cause many replication conflicts.
No.
For the sake of replication master consistency being able to run the same configuration on all masters is extremely important.
yes, but you're argument about repl conflicts is not correct
Finally, any delays in refint/memberof processing:
- We have a better MO algorithm there, it's just not been implemented
yet.
- Perhaps refint is missing indexes, or needs an algorithmic change of
it's own.
I think that it's a better investment of time to *fix* our problems, rather than hide and delay them behind async processing tasks (which arguably, will cause other random delays by being batched, and holding the write lock for a longer time period than a single write).
I fully agree. but there is no need for investment for a delay in refint, it is there and is used by a few deployments - and you want to invest into takeing this away and/or replcaing it while there is no need or requirement to do so
So, I will open this ticket, and I think that given your concerns about this Ludwig, we'll need to keep in mind that refint processing performance is a key aspect of this change, and that we will need to perform some significant load testing to guarantee we will not cause a regression in this space.
389-devel mailing list -- 389-devel@lists.fedoraproject.org To unsubscribe send an email to 389-devel-leave@lists.fedoraproject.org
On 07/07/2017 04:44 AM, Ludwig Krispenz wrote:
On 07/07/2017 07:10 AM, William Brown wrote:
Any thoughts or objections on the above would be welcome.
The only problem with going to a queue is if the server goes down unexpectedly. In such a case those RI updates would be lost.
We already have this issue because there is a delay between the change to the object and the log being sync() to disk. So we can already lose changes here. TBH the only fix is ot remove the async model. I actually question why we still need async/delay processing of the refint plugin ...
Historically speaking, a long time ago, we used to see high CPU when the RI plugin was engaged. Setting the delay to 1 second, and allowing the log thread to do the work, improved performance. Of course this is now obsolete with the betxn plugin model and other improvements, but I wanted to share why the feature even existed.
I guess that would be related to internal op searches / lack of indexing. These days it's not as big of an issue.
boldly said. How do you know, did you verify it ?
Sorry I didn't realize there was still an issue here. Back in NDS 3.12 (maybe early 4.0) this was a very common problem (I personally handled countless cases on it), but I have not seen it reported since then. If it's still useful, then we should definitely keep it, and moving to a queue is fine with me if it really adds value. I really thought the log/delay was obsolete and no one used it anymore. It still breaks the be txn model, so moving it back to a plain postop plugin is the right move if we continue to use the delay. In that case, should we make the delay the default? If it's a still a common problem as you said, then lets just apply the "fix/delay" out of the box.
we have seen many customer issues with refereint which were resolved by making it async, just removing this option without proof of a better solution is no good. I also am not sure if we need to tie anything into the betxn. There are operations, which, in my opinion, can be delayed, even redone by fixup tasks, so it is not necessary to have it in one txn, and if the option is there to delay it if you want, we should not take it away
So, lets open a ticket to remove delayed processing mode then?
you can, but I will oppose to implement it :-)
389-devel mailing list -- 389-devel@lists.fedoraproject.org To unsubscribe send an email to 389-devel-leave@lists.fedoraproject.org
-- Red Hat GmbH, http://www.de.redhat.com/, Registered seat: Grasbrunn, Commercial register: Amtsgericht Muenchen, HRB 153243, Managing Directors: Charles Cachera, Michael Cunningham, Michael O'Neill, Eric Shander
389-devel mailing list -- 389-devel@lists.fedoraproject.org To unsubscribe send an email to 389-devel-leave@lists.fedoraproject.org
On 07/07/2017 03:17 PM, Mark Reynolds wrote:
On 07/07/2017 04:44 AM, Ludwig Krispenz wrote:
On 07/07/2017 07:10 AM, William Brown wrote:
> Any thoughts or objections on the above would be welcome. The only problem with going to a queue is if the server goes down unexpectedly. In such a case those RI updates would be lost.
We already have this issue because there is a delay between the change to the object and the log being sync() to disk. So we can already lose changes here. TBH the only fix is ot remove the async model. I actually question why we still need async/delay processing of the refint plugin ...
Historically speaking, a long time ago, we used to see high CPU when the RI plugin was engaged. Setting the delay to 1 second, and allowing the log thread to do the work, improved performance. Of course this is now obsolete with the betxn plugin model and other improvements, but I wanted to share why the feature even existed.
I guess that would be related to internal op searches / lack of indexing. These days it's not as big of an issue.
boldly said. How do you know, did you verify it ?
Sorry I didn't realize there was still an issue here. Back in NDS 3.12 (maybe early 4.0) this was a very common problem (I personally handled countless cases on it), but I have not seen it reported since then. If it's still useful, then we should definitely keep it, and moving to a queue is fine with me if it really adds value. I really thought the log/delay was obsolete and no one used it anymore.
well, the latest issue I remember was on RHEL6.x with 1.2.10/11, but if we didn't hear it could als very well mean that it is used by recommendation of support. That is why I asked if it is confirmed that there is no more problem, as far as I know we did only change configuration, nothing in the plugin itself. Why do we want to fix soemthing which is not broken ? The delay option is there forever, some deployments are using it, otehrs use the default. And I would als say any effort to optimize it by using a queue is not justified, nobody complained about peformance in delay mode.
I would also not move it away from betxn, if someone uses the delay option, the update is outside the txn, that is a deliberate decision. But for all the deployments using the default delay=0 it has to stay where it is.
It still breaks the be txn model, so moving it back to a plain postop plugin is the right move if we continue to use the delay. In that case, should we make the delay the default? If it's a still a common problem as you said, then lets just apply the "fix/delay" out of the box.
we have seen many customer issues with refereint which were resolved by making it async, just removing this option without proof of a better solution is no good. I also am not sure if we need to tie anything into the betxn. There are operations, which, in my opinion, can be delayed, even redone by fixup tasks, so it is not necessary to have it in one txn, and if the option is there to delay it if you want, we should not take it away
So, lets open a ticket to remove delayed processing mode then?
you can, but I will oppose to implement it :-)
389-devel mailing list --389-devel@lists.fedoraproject.org To unsubscribe send an email to389-devel-leave@lists.fedoraproject.org
-- Red Hat GmbH,http://www.de.redhat.com/, Registered seat: Grasbrunn, Commercial register: Amtsgericht Muenchen, HRB 153243, Managing Directors: Charles Cachera, Michael Cunningham, Michael O'Neill, Eric Shander
389-devel mailing list --389-devel@lists.fedoraproject.org To unsubscribe send an email to389-devel-leave@lists.fedoraproject.org
389-devel@lists.fedoraproject.org
-
Ilias Stamatis -
Ludwig -
Ludwig Krispenz
-
Mark Reynolds -
William Brown