Christopher J. Morrone
2011-May-06 21:53 UTC
[Lustre-devel] Technical debt in the lustre build system
Eric Barton has been raising awareness about the need to address technical debt in the lustre code base. I think we should also start talking about the technical debt in the lustre build system. Overtime, we''ve cobbled together a very complex, and very fragile build system for lustre. Every time I work on the build system, my frustration level builds and I am tempted to pull the whole thing apart and start from scratch. But when I am thinking more rationally, I admit that a more evolutionary approach to improving the build system would be more likely to succeed. So I''d like to start a discussion about where we need to go with the build system. Here are some of the things off the top of my head that are problems that need to be addressed, or improvements that I think we should make. 1) The recursive configure system should be removed. Each system that requires its own build system should be a standalone package. Each standalone package should have proper Requires:, BuildRequires:, Provides:, etc. in the .spec file for rpm. Appropriate equivalents should be used in other packaging systems (deb). ldiskfs is a good candidate for this. With the changes to support multiple backend filesystems, making the backends separate packages makes even more sense than it did in the past. In fact, LLNL has already packaged ldiskfs separately for 2.1. It would be great if the rest of the community adopted this approach in a future release. I am guessing that the snmp directory could easily be its own package as well. Lets identify more things like that. 2) Installed files need serious cleanup and reorganization. Case in point, the main lustre package installs this file: /usr/bin/config.sh This pretty much wins the lifetime award for Poorly Named Command In A Standard Path Location. There are many others such as obdfilter-survey, ost-survey, parse-ior, plot-obdfilter, etc. that are clearly useful testing tools, but inappropriate for the main lustre rpm package. 3) Remove old build system tools dealing with CVS or Subversion repositories. We''ve moved to git, and it is clearly superior. We are not going back. It is time to remove the cruft. 4) make_META.pl -> version_tag.pl. Why is make_META.pl part of the build system and just a symlink to version_tag.pl? I don''t understand the rationale on this one. Mighty confusing when you need to fix a bug in make_META.pl, but no file named make_META.pl exists in your source tree. 5) Need to keep in mind that third parties will be building this, and will need the flexibility to have their own tags and versioning schemes. We can partly do this now, but it needs improvement. Some of the code to check git version numbers and tags and such seems like it was well intentioned, but just adds too much complexity to an already complex problem. Lets look into ways to simplify this. 6) The lustre.spec file. Lets face it, rpm''s spec language is just awful. But it is what we are stuck with for most of our platforms, so we need to figure out how to live with it. Lustre''s spec file is a bit of a mess now, and pretty difficult for those of us downstream to use unmodified. Some of the previous suggestions will naturally improve the state of the spec file, but additional improvements are needed. I think we should take another look at the decision to parse --with-linux and --with-linux-objs out of %configure_args. It just makes the interactions between various rpm variables and configure arguments too complex, in my opinion. I think that we can take some inspiration here from Brian Behlendorf''s zfs-modules.spec.in file in his ZFS repo: https://github.com/behlendorf/zfs Brian has gone to great lengths to make ZFS buildable under just about every Linux distro under the sun, and I still am able to understand his spec file. I can''t say the same for Lustre''s spec file, and lustre doesn''t build nearly as cleanly. Grantly, lustre is a bit more complex in ways...but by splitting the code into multiple projects I think we can reduce the spec file complexity. 7) build/lbuild-* What is this stuff? Does anyone outside of the core CFS/Sun/Oracle/etc. team use this? Seriously, if you do, please speak up. I know that LLNL has never used it. Frankly, I think it should be removed from the main Lustre tree. My impression, from a brief skimming of the files, is that they are the automated build system that upstream has used to generate kernel packages, lustre packages, and maybe IB packages. LLNL uses an automated build environment based on buildbot that builds lustre and all of our other packages under a chroot environment individually created for each package by "mock". It contains only the rpms needed by the package, which enforces that we have to have our spec file dependencies correct (another reason why the lustre.spec often doesn''t work for us). That is a bit of a digression, but my point is this: we probably all have our own build systems to contend with. Those scripts shouldn''t be part of the main lustre tree. They should be a separate package, or just Whamcloud''s internal scripts if no one else is using them. 8) Lustre .src.rpm should be rebuildable. It is now, more-or-less, but could use improvement. So where do we go from here? I think we should set up a wiki page to plan the overhaul, and start opening bugs to track individual changes that need to be made. Make a large overhaul for 2.1 is out of the question, but perhaps we can make many of the changes in the next release. Chris
Ken Hornstein
2011-May-10 02:53 UTC
[Lustre-devel] Technical debt in the lustre build system
>So I''d like to start a discussion about where we need to go with the >build system. Here are some of the things off the top of my head that >are problems that need to be addressed, or improvements that I think we >should make. >[...]I agree with everything you''ve said, but I''d like to add one thing: 9) Portability This drives me nuts on the Mac port; the build system has a fair amount of cruft left over (see autoMakefile versus Makefile.am, for starters). If we''re thinking about long-term plans on the build system, thinking about portability is important. (On the Mac, and AFAIK all other operating systems other than Linux, building a kernel module is relatively straightforward; just compile everything with a few extra options). --Ken
Andreas Dilger
2011-May-10 21:53 UTC
[Lustre-devel] Technical debt in the lustre build system
On 2011-05-06, at 3:53 PM, "Christopher J. Morrone" <morrone2 at llnl.gov> wrote:> Eric Barton has been raising awareness about the need to address > technical debt in the lustre code base. I think we should also start > talking about the technical debt in the lustre build system. > > Overtime, we''ve cobbled together a very complex, and very fragile build > system for lustre. Every time I work on the build system, my > frustration level builds and I am tempted to pull the whole thing apart > and start from scratch. But when I am thinking more rationally, I admit > that a more evolutionary approach to improving the build system would be > more likely to succeed. > > So I''d like to start a discussion about where we need to go with the > build system. Here are some of the things off the top of my head that > are problems that need to be addressed, or improvements that I think we > should make.Chris, I tend to agree with most of your statements. Having a simpler build system is desirable for everyone. It also makes sense to have "make rpms" use this build system instead of having a separate system to handle the "production" build vs "homebrew" builds, which isn''t the case today. I think it would be great to see small incremental patches that fix the problems that you have detailed here. Some of them appear to be very minor changes (i.e. extra files included in the RPM packages, or poorly-named files). Other changes are more extensive, and lumping them all together would mean that accepting the simple changes is blocked behind testified and landing the large changes, which is going to be slower. Ken also mentioned the Makefile vs. autoMakefile.am issue, and this is a historic artifact of when Lustre built on both 2.4 and 2.6 kernels, and is no longer needed. It might still make sense to have a simple "list of source files" that can be included by the various Makefiles for each platform, so that there isn''t a need to modify 3 or 4 makefiles whenever a new source file is added.> 1) The recursive configure system should be removed. > > Each system that requires its own build system should be a standalone > package. Each standalone package should have proper Requires:, > BuildRequires:, Provides:, etc. in the .spec file for rpm. Appropriate > equivalents should be used in other packaging systems (deb). > > ldiskfs is a good candidate for this. With the changes to support > multiple backend filesystems, making the backends separate packages > makes even more sense than it did in the past. In fact, LLNL has > already packaged ldiskfs separately for 2.1. It would be great if the > rest of the community adopted this approach in a future release.One issue with separating ldiskfs into it''s own package is that fsfilt (or obd-ldiskfs on newer versions of Lustre) are linked closely to the ldiskfs code, and cannot completely be configured using the header file today. That said, it may be practical to store the results of the configure check in the ldiskfs header itself (e.g. #define HAVE_SOME_FEATURE) but this would also need a bit of work. I''d be interested to see how this is handled by the LLNL build system today.> I am guessing that the snmp directory could easily be its own package as > well. > > Lets identify more things like that. > > 2) Installed files need serious cleanup and reorganization. Case in > point, the main lustre package installs this file: > > /usr/bin/config.sh > > This pretty much wins the lifetime award for Poorly Named Command In A > Standard Path Location. There are many others such as obdfilter-survey, > ost-survey, parse-ior, plot-obdfilter, etc. that are clearly useful > testing tools, but inappropriate for the main lustre rpm package. > > 3) Remove old build system tools dealing with CVS or Subversion > repositories. We''ve moved to git, and it is clearly superior. We are > not going back. It is time to remove the cruft.Sure, I don''t think anyone disagrees. There were also old config scripts and tests that could be removed, I''m not sure if they have been or not.> 4) make_META.pl -> version_tag.pl. Why is make_META.pl part of the > build system and just a symlink to version_tag.pl? I don''t understand > the rationale on this one. Mighty confusing when you need to fix a bug > in make_META.pl, but no file named make_META.pl exists in your source tree. > > 5) Need to keep in mind that third parties will be building this, and > will need the flexibility to have their own tags and versioning schemes. > We can partly do this now, but it needs improvement. > > Some of the code to check git version numbers and tags and such seems > like it was well intentioned, but just adds too much complexity to an > already complex problem. Lets look into ways to simplify this. > > 6) The lustre.spec file. > > Lets face it, rpm''s spec language is just awful. But it is what we are > stuck with for most of our platforms, so we need to figure out how to > live with it. Lustre''s spec file is a bit of a mess now, and pretty > difficult for those of us downstream to use unmodified. Some of the > previous suggestions will naturally improve the state of the spec file, > but additional improvements are needed. I think we should take another > look at the decision to parse --with-linux and --with-linux-objs out of > %configure_args. It just makes the interactions between various rpm > variables and configure arguments too complex, in my opinion. > > I think that we can take some inspiration here from Brian Behlendorf''s > zfs-modules.spec.in file in his ZFS repo: > > https://github.com/behlendorf/zfs > > Brian has gone to great lengths to make ZFS buildable under just about > every Linux distro under the sun, and I still am able to understand his > spec file. I can''t say the same for Lustre''s spec file, and lustre > doesn''t build nearly as cleanly. > > Grantly, lustre is a bit more complex in ways...but by splitting the > code into multiple projects I think we can reduce the spec file complexity.I agree. However, you also need to recognize that Lustre was started when RHEL3 was the main distro, so the ability of the RPM .spec file has increased significantly since that time. I don''t mean to indicate that it _shouldn''t_ be cleaned up, but it hasn''t taken a priority to date, if it continues to work.> 7) build/lbuild-* > > What is this stuff? Does anyone outside of the core CFS/Sun/Oracle/etc. > team use this? Seriously, if you do, please speak up. > > I know that LLNL has never used it. Frankly, I think it should be > removed from the main Lustre tree. My impression, from a brief skimming > of the files, is that they are the automated build system that upstream > has used to generate kernel packages, lustre packages, and maybe IB > packages. > > LLNL uses an automated build environment based on buildbot that builds > lustre and all of our other packages under a chroot environment > individually created for each package by "mock". It contains only the > rpms needed by the package, which enforces that we have to have our spec > file dependencies correct (another reason why the lustre.spec often > doesn''t work for us). > > That is a bit of a digression, but my point is this: we probably all > have our own build systems to contend with. Those scripts shouldn''t be > part of the main lustre tree. They should be a separate package, or > just Whamcloud''s internal scripts if no one else is using them. > > 8) Lustre .src.rpm should be rebuildable. It is now, more-or-less, but > could use improvement. > > So where do we go from here? I think we should set up a wiki page to > plan the overhaul, and start opening bugs to track individual changes > that need to be made. > > Make a large overhaul for 2.1 is out of the question, but perhaps we can > make many of the changes in the next release. > > Chris
Christopher J. Morrone
2011-May-10 23:24 UTC
[Lustre-devel] Technical debt in the lustre build system
On 05/09/2011 07:53 PM, Ken Hornstein wrote:>> So I''d like to start a discussion about where we need to go with the >> build system. Here are some of the things off the top of my head that >> are problems that need to be addressed, or improvements that I think we >> should make. >> [...] > > I agree with everything you''ve said, but I''d like to add one thing: > > 9) Portability > > This drives me nuts on the Mac port; the build system has a fair amount > of cruft left over (see autoMakefile versus Makefile.am, for starters). > If we''re thinking about long-term plans on the build system, thinking > about portability is important. > > (On the Mac, and AFAIK all other operating systems other than > Linux, building a kernel module is relatively straightforward; > just compile everything with a few extra options).Yeah, that would be nice. It does seem odd at first glance that most directories have both autoMakefile.am files and Makefile.in files. I think that what is going on there is are essentially two independent build system: normal user-space build stuff (liblustre) uses the autoMakefile, and the kernel build system uses Makefile. But in the end, Makefile does an "include autoMakefile", making one wonder why they need to be separate in the first place. It is certainly worth investigating to see if that can be improved to be both more understandable and more portable. Chris
Christopher J. Morrone
2011-May-11 18:05 UTC
[Lustre-devel] Technical debt in the lustre build system
On 05/10/2011 02:53 PM, Andreas Dilger wrote:> Chris, I tend to agree with most of your statements. Having a simpler build system is desirable for everyone. It also makes sense to have "make rpms" use this build system instead of having a separate system to handle the "production" build vs "homebrew" builds, which isn''t the case today.Agreed!> I think it would be great to see small incremental patches that fix the problems that you have detailed here. Some of them appear to be very minor changes (i.e. extra files included in the RPM packages, or poorly-named files).I totally agree; I definitely think this needs an incremental approach. But also a concerted effort to drive those incremental changes. LLNL has tried to push up patches to clean up some of those minor changes in the past and failed to gain traction. Maybe we just didn''t push hard enough. So this time I want to keep pushing. And get more people involved so hopefully they''ll understand where the incremental changes are leading.> Ken also mentioned the Makefile vs. autoMakefile.am issue, and this is a historic artifact of when Lustre built on both 2.4 and 2.6 kernels, and is no longer needed. It might still make sense to have a simple "list of source files" that can be included by the various Makefiles for each platform, so that there isn''t a need to modify 3 or 4 makefiles whenever a new source file is added.Great.> One issue with separating ldiskfs into it''s own package is that fsfilt (or obd-ldiskfs on newer versions of Lustre) are linked closely to the ldiskfs code, and cannot completely be configured using the header file today. That said, it may be practical to store the results of the configure check in the ldiskfs header itself (e.g. #define HAVE_SOME_FEATURE) but this would also need a bit of work. > > I''d be interested to see how this is handled by the LLNL build system today.Sure, I''ll get Ned to comment on this.>> Grantly, lustre is a bit more complex in ways...but by splitting the >> code into multiple projects I think we can reduce the spec file complexity. > > I agree. However, you also need to recognize that Lustre was started when RHEL3 was the main distro, so the ability of the RPM .spec file has increased significantly since that time. I don''t mean to indicate that it _shouldn''t_ be cleaned up, but it hasn''t taken a priority to date, if it continues to work.Sure, I understand that. Though you may be giving newer versions of RPM too much credit. :) Chris
Christopher J. Morrone
2011-May-11 18:22 UTC
[Lustre-devel] Technical debt in the lustre build system
On 05/11/2011 11:05 AM, Christopher J. Morrone wrote:> On 05/10/2011 02:53 PM, Andreas Dilger wrote:>>> One issue with separating ldiskfs into it''s own package is that fsfilt (or obd-ldiskfs on newer versions of Lustre) are linked closely to the ldiskfs code, and cannot completely be configured using the header file today. That said, it may be practical to store the results of the configure check in the ldiskfs header itself (e.g. #define HAVE_SOME_FEATURE) but this would also need a bit of work. >> >> I''d be interested to see how this is handled by the LLNL build system today. > > Sure, I''ll get Ned to comment on this.Ned''s email got stuck in moderation. Here''s a copy: "We create a package lustre-ldiskfs-devel that provides all of the ldiskfs headers including ldiskfs_extents.h and ldiskfs_jbd2.h. I believe those two along with ldiskfs.h are sufficient to configure fsfilt (though the Lustre build system would need to be updated to include them instead of their ext4 equivalents). That said, I like your suggestion to have ldiskfs directly export its configuration via ldiskfs.h. Ned"
Ashley Pittman
2011-May-12 08:58 UTC
[Lustre-devel] Technical debt in the lustre build system
On 6 May 2011, at 23:53, Christopher J. Morrone wrote:> 2) Installed files need serious cleanup and reorganization. Case in > point, the main lustre package installs this file: > > /usr/bin/config.sh > > This pretty much wins the lifetime award for Poorly Named Command In A > Standard Path Location. There are many others such as obdfilter-survey, > ost-survey, parse-ior, plot-obdfilter, etc. that are clearly useful > testing tools, but inappropriate for the main lustre rpm package.You could argue that this is a separate issue from the build but I can see it''d be easier to fix them both at the same time.> 5) Need to keep in mind that third parties will be building this, and > will need the flexibility to have their own tags and versioning schemes. > We can partly do this now, but it needs improvement.We rebuild Lustre and add our own tag to the build, if it''s not possible to add a tag to the build on the command line we''ll need to patch it ourselves.> 7) build/lbuild-* > > What is this stuff? Does anyone outside of the core CFS/Sun/Oracle/etc. > team use this? Seriously, if you do, please speak up.> I know that LLNL has never used it. Frankly, I think it should be > removed from the main Lustre tree. My impression, from a brief skimming > of the files, is that they are the automated build system that upstream > has used to generate kernel packages, lustre packages, and maybe IB > packages.We use this right now. We are not tied to it however we need some of the functionality it provides so if we move away from it we''ll have to find another way.> That is a bit of a digression, but my point is this: we probably all > have our own build systems to contend with. Those scripts shouldn''t be > part of the main lustre tree. They should be a separate package, or > just Whamcloud''s internal scripts if no one else is using them.We''d be happy to maintain our own if it meant the central tree could be cleaner and easier to use for people using the stock release.> So where do we go from here?10) One of the things that bugs me is that currently we build and distribute a new kernel for each and every update when in theory we could re-use the kernel and only update the modules 90% of the time. As a background to this we maintain our list of patches to Lustre with quilt and for any given commit there is no way that I know of to tell if the change impacts the kernel patches or just the module/userspace. As a result the only safe thing to do is to build a whole new kernel each and every time. Actually the cost of this is pretty low as Lustre runs on dedicated machines so rebooting during updates is not an issue. One thing I changed at Quadrics where we had a similar problem was to separate the kernel patches from the kernel modules into different "packages" and maintain, distribute, and update them as separate bits of software. This had the added benefit of making it obvious when people were patching the kernel further which reduced the incidence of this and meant we put more thought into changes. I suspect this wouldn''t work for Lustre as it''s more intrusive into the kernel source but I''d welcome ideas for solving the do-I-update-the-kernel-or-not problem. Ashley.
Christopher J. Morrone
2011-May-12 17:28 UTC
[Lustre-devel] Technical debt in the lustre build system
On 05/12/2011 01:58 AM, Ashley Pittman wrote:> 10) > > One of the things that bugs me is that currently we build and distribute a new kernel for each and every update when in theory we could re-use the kernel and only update the modules 90% of the time. As a background to this we maintain our list of patches to Lustre with quilt and for any given commit there is no way that I know of to tell if the change impacts the kernel patches or just the module/userspace. As a result the only safe thing to do is to build a whole new kernel each and every time. Actually the cost of this is pretty low as Lustre runs on dedicated machines so rebooting during updates is not an issue.I feel for you. We used to maintain our patches to lustre in quilt, but we''ve moved to just keeping a branch in git. I for one am MUCH happier now that we''ve eliminated that quilt part.> One thing I changed at Quadrics where we had a similar problem was to separate the kernel patches from the kernel modules into different "packages" and maintain, distribute, and update them as separate bits of software. This had the added benefit of making it obvious when people were patching the kernel further which reduced the incidence of this and meant we put more thought into changes. I suspect this wouldn''t work for Lustre as it''s more intrusive into the kernel source but I''d welcome ideas for solving the do-I-update-the-kernel-or-not problem.Yes, I agree, that is an issue. We maintain our own kernel release, and currently we need to keep the same watch on the kernel_patches directories that you do so that we can regularly pull the patches into our kernel repo. I think we can greatly improve that situation. I see two things that would make a big difference: 1) Make lustre "patchless". If you ignore the ldiskfs patches, the kernel patch set for lustre is getting quite small. There is even the belief that with some effort lustre can go completely patchless (ignoring ldiskfs) for the server as well as the client. This effort is under way, and there are bugs open to track it. But it is one of those items that tends to have slow progress because the developers are busy with other tasks. 2) Package ldiskfs separately. Make direct commits to ldiskfs''s git repo rather than maintaining a quilt patch stack. Chris
>> I think it would be great to see small incremental patches that fix the problems that you have detailed here. Some of them appear to be very minor changes (i.e. extra files included in the RPM packages, or poorly-named files). > I totally agree; I definitely think this needs an incremental approach. > But also a concerted effort to drive those incremental changes. > > LLNL has tried to push up patches to clean up some of those minor > changes in the past and failed to gain traction. Maybe we just didn''t > push hard enough. So this time I want to keep pushing. And get more > people involved so hopefully they''ll understand where the incremental > changes are leading. >Perhaps the right approach is to create a specification or document that fully outlines what the objective is and get sign-up from the broader community before work is started. The incremental changes can then refer to the ''master document'' to illustrate the reason for the change. Little steps are a good way of making sure that we don''t trip over, but without an agreed long term target they don''t allow people to be sure where the changes are taking them. Chris --- Chris Gearing Snr Engineer Whamcloud. Inc. -------------- next part -------------- An HTML attachment was scrubbed... URL: http://lists.lustre.org/pipermail/lustre-devel/attachments/20110512/6b4bdb30/attachment.html