In Aegir (and with Drush enthousiasts), you can end up with a lot of aliases in your ~/.drush directory. In fact, in Aegir, you have an alias for every single site out there, which is very convenient, but can also be veeeery slow if you have a lot of sites, because of a bug in the table output routines (see #1132902: performance issues with huge tables). Besides, it makes the output of "core-status" really cluttered and incomprehensible as drush attempts to show you all your alias files, the actual info gets lost in the backlog.

I think we should instead "lazy-load" the aliases, just on demand. Basically, we need to optimize _drush_sitealias_find_and_load_alias() so that it doesn't require all those aliases when we request a single alias to be loaded. There's a drush_sitealias_load_all() function to load all aliases, find_and_load() shouldn't need to load them all.

Members fund testing for the Drupal project. Drupal Association Learn more

Comments

greg.1.anderson’s picture

Assigned: Unassigned » greg.1.anderson

I'm going to have to look at the code again; it's only supposed to load all aliases if you do a drush sa call or something like that. By design, it did (at least at one time) lazy-load single-alias alias files.

greg.1.anderson’s picture

So, I looked at the code, and indeed, it does lazy-load *.alias.drushrc.inc files, but I guess the problem you are having is that core-status explicitly calls drush_sitealias_load_all() so that it can call drush_get_context('drush-alias-files') to list all of your alias files.

So, this would be easy to fix; one possible solution would be to just remove the call to ...load_all() in core-status and not show any alias files in the core status output. However, this solution is a little dissatisfying, as showing the alias files by default is very useful when you only have a few alias files.

We could improve the performance of core-status a little by factoring a ...find_all() function out of ..load_all(), and call ...find_all() from core-status to avoid loading alias files in an instance where all you really care about is the names of the file.

That would not solve the performance problem of console table when there are a large number of items in a cell (per the above-referenced incident). I'm unsure what the best solution is here, but perhaps if ...find_all() returns more than a certain number of files, the output could list just the first few and then add "and 596 more", or something like that to the status table, perhaps with an option to show all. There are probably other ways to tackle it as well.

anarcat’s picture

I don't see why drush @site status shows unrelated aliases. The actual performance problem here is actually #1132902: performance issues with huge tables, not the actual loading of aliases. So maybe if we just strip the display or display only relevant aliases, this would be fixed?

Otherwise, we can just close this issue if aliases are really lazy-loaded...

greg.1.anderson’s picture

Hm, maybe we should only call ...load_all() if no site is bootstrapped, so drush status shows you all of your aliases (presuming cwd is not inside a site), and drush @site status shows you only the alias files loaded to find @site?

anarcat’s picture

I wouldn't even call _load_all() for drush status - we have drush sa for that. But yeah, that would be a good start. :)

greg.1.anderson’s picture

Drush sa does not show you what files the aliases come from.

anarcat’s picture

maybe that should be fixed then. :)

moshe weitzman’s picture

In general, it is handy to know what file X comes from. I sometimes wonder that with commands as well.

Sounds good to me: "the output could list just the first few and then add 'and 596 more'"

greg.1.anderson’s picture

Status: Active » Needs review
FileSize
2.13 KB

Here's a patch that:

1. behaves the same as always for < 24 alias files
2. prints only the number of aliases for 24+ alias files
3. prints all the aliases regardless of how many there are if --full is specified with the status command

n.b. drush sa does now show you what file each alias comes from (in --full mode, anyway).

This bikeshed is as painted as it's going to get, as far as I am concerned. If anyone with lots and lots of aliases would like to adjust the hue, that's fine by me, just put it in a patch. It's fine with me to commit with whatever variation is deemed good (e.g. I didn't see any value of showing just three aliases out of 200, but it's fine by me if it does that).

anarcat’s picture

I wonder: does this fix the actual issue reported? :) I have *assumed* that the bug was due to a performance problem in the table rendering routine, but I could be totally wrong about this...

Other than that, I think the shed should be blue, of course: http://blue.bikeshed.org/ ;)

greg.1.anderson’s picture

Status: Needs review » Postponed (maintainer needs more info)

Need feedback from OP on whether this fixes the issue reported. If not, patches welcome. :)

In another issue we discussed options for removing the Drush info from the status report; maybe that might be another avenue to pursue. #1382352: Modal configuration options via --context-prefix. provides a --site-only option, and a way to automatically set it when running 'core-status' on multiple sites. Presumably, you would need only the first half of that patch.

anarcat’s picture

Crap. *I* am the OP. :P Problem is: Aegir is not ported to Drush 5 yet, so I can't really test this easily, short of copying alias files around...

That will teach me to open my big mouth. ;)

moshe weitzman’s picture

Status: Postponed (maintainer needs more info) » Reviewed & tested by the community

Looks like a UI fix, regardless of whether it speeds anything up.

greg.1.anderson’s picture

I'm fine with committing this as-is, and adjusting future concerns in future patches. Heck, we could even mark this as patch-to-be-ported, and try it out there -- at least as a UI change. Drush-4 still uses drush_sitealias_load_all() and drush_get_context('drush-alias-files')) to get the alias list, but the concept is exactly the same, since you still end up with an array of aliases in the end. Performance may vary, but will be better in Drush-5 than in Drush-4. The fact that we already replaced drush_sitealias_load_all() in Drush-5 is part of what makes me think this patch will largely solve the problem in 5.

The issue quoted in #4 (#1382352: Modal configuration options via --context-prefix.) would definitely help performance, though, as it would completely bypass alias loading the alias list when --site-only is specified. The --site-only portion of that patch, at least, could be backported to Drush-4.

greg.1.anderson’s picture

Version: » All-versions-4.x-dev
Status: Reviewed & tested by the community » Needs review
FileSize
1.84 KB

Committed to master. Here's a patch for Drush-4, if anyone wants to try it out; could go in there too, if desired.

greg.1.anderson’s picture

For Drush-4; commit or close.

moshe weitzman’s picture

Version: All-versions-4.x-dev »
Status: Needs review » Fixed

Guess we are not taking this.

Automatically closed -- issue fixed for 2 weeks with no activity.