Bug 234177 - Repository references not working in p2 metadata repositories
Summary: Repository references not working in p2 metadata repositories
Status: RESOLVED FIXED
Alias: None
Product: Equinox
Classification: Eclipse Project
Component: p2 (show other bugs)
Version: 3.4   Edit
Hardware: PC Windows XP
: P3 normal (vote)
Target Milestone: 3.4 RC3   Edit
Assignee: John Arthorne CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-05-27 11:29 EDT by John Arthorne CLA
Modified: 2008-05-30 08:42 EDT (History)
3 users (show)

See Also:
pascal: review+
susan: review+


Attachments
Fix (1.14 KB, patch)
2008-05-27 11:51 EDT, John Arthorne CLA
no flags Details | Diff
Updated patch (2.43 KB, patch)
2008-05-28 17:10 EDT, John Arthorne CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description John Arthorne CLA 2008-05-27 11:29:38 EDT
Build: 3.4 RC2

Local metadata repositories are capable of containing references to other repositories, but this support is not working. If I add the Ganymede update site, its referenced repositories are not added to the list of available repositories.
Comment 1 John Arthorne CLA 2008-05-27 11:51:26 EDT
Created attachment 102180 [details]
Fix

This fixes the problem, but reveals some new problems. In particular, StructuredViewerProvisioningListener invokes a refresh of the view for each repository discovery event, and since the Ganymede site has 62 references, it causes 62 refreshes of the tree and the UI goes berserk. Also, a large number of these sites are enabled by default because they are feature update sites. After seeing how this plays on a large scale, I think all repository references should be disabled by default and the user should have to manually enable them.
Comment 2 Susan McCourt CLA 2008-05-27 14:17:05 EDT
I agree this doesn't scale well.  

I just loaded the patch and when I opened the available features view, I suddenly saw a bunch of sites get added (presumably because I had Ganymede already in my site list and all the new sites were being added).  The scroll bar thumb was shrinking before my eyes, and then I tried to remove the ganymede site in the manage sites dialog.  That dialog was also exploding with new sites added, and I had a ton of download jobs running, obtaining different jars (not sure why that part was happening).

The overall experience was quite disconcerting.

>Also, a large number
>of these sites are enabled by default because they are feature update sites.
>After seeing how this plays on a large scale, I think all repository references
>should be disabled by default and the user should have to manually enable them.

This is probably the best we can do for 3.4, but unfortunate for the UM users who never had to be all that aware of where feature updates came from.  I've opened bug 234213 to separately discuss how we are going to deal with this post 3.4.
Comment 3 Susan McCourt CLA 2008-05-27 14:36:29 EDT
This patch also exacerbates the performance problems in bugs like 234180, because NLS string lookup is visiting 62 more repositories.  I just added Ganymede update site with some of my experiments for bug #229069, and although my patches significantly reduce the refresh dance, and avoid automatic loading of all of the new repos, the next time I tried to expand a ganymede category, the UI hung while it was looking in all the new and not loaded yet repos. 
Comment 4 Susan McCourt CLA 2008-05-28 16:09:10 EDT
I previously said:
>I just added Ganymede update site with some of my experiments for bug #229069, >and although my patches significantly reduce the refresh dance, and avoid >automatic loading of all of the new repos...

The experiments I was referring to in the end did not fix the bug, so I removed them.  This means that we will have a refresh dance when the patch for this bug is released.  The viewer will refresh for every referred repo that gets added.

I opened bug 234489 to discuss the refresh issue but did not put a milestone on it.  
Comment 5 John Arthorne CLA 2008-05-28 17:10:59 EDT
Created attachment 102520 [details]
Updated patch

This new patch adds the fix to the generator to avoid enabling feature update sites by default. We think this change is necessary, otherwise there are just too many extra sites added automatically when a site is first loaded. This leads to a poor experience in the UI where expanding a repository causes more repositories to be added, or when you first type in the filter field it will go off and load repositories, which causes references to be added, and then they have to be loaded, etc. Similarly this will cause problems in headless contexts, where the list of "known repositories" will change after repositories are loaded for the first time.
Comment 6 John Arthorne CLA 2008-05-28 17:15:57 EDT
Note there were three possible approaches to addressing the scalability problem mentioned in my first comment and Susan's comments:

1) Manually hack the ganymede update site to disable references, since this is the worst case. I don't think this is a good solution since there will be many other open source and corporate update sites with a large number of features, and it's hard to argue that the current behaviour is unacceptable for ganymede but acceptable for others.

2) Fix the problem at generation time by producing disabled references for feature update sites. This is what the patch does. This approach allows someone to manually tweak their site after generation if they really want the references to be enabled.

3) Fix the problem at load-time by broadcasting "disabled" discovery events for all repository references. This would alter the behaviour for both associate sites and feature update sites since we can't distinguish the two at this point. This approach would eliminate the possibility of added enabled repository references for everybody.
Comment 7 John Arthorne CLA 2008-05-28 17:50:20 EDT
David, once this patch is reviewed, we are going to have a chicken/egg problem. We can't release the patch in comment #1 until the Ganymede site is rebuilt with the generator fix in place (we have been testing and the performance is just too bad). When is the next update of /releases/ganymede/ scheduled? Will it be possible to regenerate and replace the content.jar this week? I could alternatively provide a patched content.jar produced by hand if needed.  

We're not ready to release yet, but just wanted to give you a heads up that some coordination will be needed.
Comment 8 David Williams CLA 2008-05-28 18:09:16 EDT
(In reply to comment #7)
> David, once this patch is reviewed, we are going to have a chicken/egg problem.
> We can't release the patch in comment #1 until the Ganymede site is rebuilt
> with the generator fix in place (we have been testing and the performance is
> just too bad). When is the next update of /releases/ganymede/ scheduled? Will
> it be possible to regenerate and replace the content.jar this week? I could
> alternatively provide a patched content.jar produced by hand if needed.  
> 
> We're not ready to release yet, but just wanted to give you a heads up that
> some coordination will be needed.
> 

/releases/ganymede currently has RC1 level next update is planned for Monday, June 2nd for RC2. 

Of course, we are producing content.jar "continuously" on 
/releases/ganymede/staging

And, yes, we can re-generate at any time. We might want to try on staging first :) 
Comment 9 Susan McCourt CLA 2008-05-28 18:20:04 EDT
+1 on the patch, conditional with Ganymede site being updated with the new disabled repo references.  Coordination is even more important given we are now adding the Ganymede site by default in the builds.

I have been running with the MetadataRepositoryIO part of the patch since yesterday, so I can verify that it works (lots of dancing in the UI, many repos getting added, etc.)  I have not seen any (new) spew in the log or other errors since running the patch, and I've connected to Ganymede, EclEmma, ECF, and the test updates site continuously and regenerating metadata, etc. in order to test my own fixes.

It's unfortunate (and will be perceived as a regression) that UM users who are used to checking for updates without doing any repo management will now have to become aware of disabled repos, but that's what bug 234213 is for.  And this patch at least makes the repos visible in the manage sites dialog, whereas they weren't known at all without the patch.
Comment 10 Pascal Rapicault CLA 2008-05-29 10:28:48 EDT
Patch reviewed and released.
Comment 11 John Arthorne CLA 2008-05-29 18:50:06 EDT
David, the fix in the generator was released for the I20080529-1300 build. If you could regenerate the Ganymede site tomorrow with that version of the generator, it will fix the problem there and avoid a poor initial experience for people grabbing platform RC3.
Comment 12 David Williams CLA 2008-05-30 00:03:12 EDT
Ok, let's review ... 

what am I supposed to see? 
I'll double check my scripts, but think I have moved the P2 generator up to I20080529-1300, and have (re) generated the 'staging' site, but when I access that staging site with I20080529-1300 build, I _still_ see it discover a whole bunch of update sites. That's supposed to be not the case, right? 

Are you sure the fix made the I20080529-1300 build? 

Comment 13 David Williams CLA 2008-05-30 00:15:17 EDT
ok, re-reading the bug, I think I misunderstood. It is still supposed to pickup/list the update sites, just not 'enable' them. 

I think that is working then. 

There is still a ton of downloads happening in the background, apparently, but maybe that's one of the other bugs listed? 

BTW, you say this not-enabled-state is the 'default' behavior. Is there a new switch to specify to the EclipseGenerator task to enable the other update sites? Or, did you must mean it is purely a client side operation now? 



Comment 14 John Arthorne CLA 2008-05-30 00:17:27 EDT
If you open the content.jar, you should see a <references> element right near the top. With the fix, they should all have "options='0'" to indicate they are disabled by default. Looking at staging now, I agree it still looks incorrect. Did you start fresh or was there an existing content.jar there already?
Comment 15 David Williams CLA 2008-05-30 00:54:45 EDT
fresh. And peeking in the file, it does have options="0"

Such as 
      9   <references size='62'>$
     10     <repository url='http://download.eclipse.org/dsdp/dd/updates' type='1' options='0'/>$
     11     <repository url='http://download.eclipse.org/dsdp/dd/updates' type='0' options='0'/>$
     12     <repository url='http://download.eclipse.org/datatools/updates' type='0' options='0'/>$
 




Comment 16 David Williams CLA 2008-05-30 03:12:23 EDT
To be explicit, I did go ahead an update 'releases' area too ... so, if someone can verify all is expected, that'd be good. 

http://download.eclipse.org/releases/ganymede/content.jar 

Comment 17 John Arthorne CLA 2008-05-30 08:42:54 EDT
The Ganymede site looks good. Thanks David!