Frédéric Camblor edited a comment on Improvement JENKINS-13613

I'm going to refactor the way the plugin is behaving concerning commits.
For the moment, it detects a change on a Saveable object and, each time, make a commit on the file concerned by this change.
You have to note that a Saveable is always attached to a file, but a file can be concerned by several Saveable (ie: descriptors)
This is the reason why, when, for instance, you persist the jenkins system configuration screen, lots of commit will be made : every plugin which has a Descriptor on system configuration data, will be saved separately => each save will make a commit.
This is the point of the issue.

What I'm going to do to address this problem :

  • I'll implement a PluginServletFilter that will initialize, for every requests, an empty Changeset in the beginning of the request. This Changeset will be stored in a ThreadLocal.
  • Then, request will be processed normally
  • Then, when request process ends, the PluginServletFilter will look at the Changeset and, if it isn't empy, it will commit every changes in this one, in only one commit.

Now, some more technical inputs :

  • Everytime a Saveable will be saved, instead of synchronizing the underlying file, current Saveable's file content will be added into the ThreadLocal's Changeset
  • A Changeset will be made of a Map<filePath, contentOfFile>. Thus, if multiple Saveable work in the same file, they will override same Changeset entry each time the Saveable is saved. I hope the byte[] won't consume too much memory...
  • This Changeset representation will avoid collision on file save :
    • 2 users edit the global jenkins configuration page
    • 1st user submit the page, it takes a long time to synchronize...
    • 2nd user submit the page, it modifies the config.xml before synchronization of 1st user is finished => changes of 2nd user will be commited during 1st user commit
  • Commit could then be made asynchronously, but this is another point (see JENKINS-14214)
  • When Saveable.save() is called "outside" an http request (triggerd by jenkins himself for instance), I won't be able to identify (for the moment) clearly a "transaction". Thus, I'll have to keep the "pre" JENKINS-13613 behaviour as a fallback : 1 commit per Saveable.save().
    On this point, though, maybe could I ask Kohsuke if he has a good entry point for batched jenkins task when I could say "hey, transaction starts here, start to track changesets on Saveable.save()"

I think by doing such a refactoring, I could fix problems like JENKINS-10967 and JENKINS-9166

This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators.
For more information on JIRA, see: http://www.atlassian.com/software/jira

Reply via email to