Community
Participate
Working Groups
Before Callisto there the biggest update site that we used to use was our own update site. This site although bigger then anything else at the time is very simple and small compared with Callisto. Features on it were much simpler then the ones you can find on Callisto. Yet even then inefficiencies in update component produced much grief to users, site webmaster and update team. Two of the biggest problems were premature downloading of features and unnecessary multiple connection openings for the same resource. To elevate first problem we introduced digest, which did help greatly with so called “first-page time”, however it did not go far enough. It did not include data for included features which caused premature downloading of included feature jars for validation purposes on review page (page where one selects features). Unnecessary multiple connection openings (up to 5 times on average per resource needed) besides bandwidth problem introduces big degree of uncertainty to do user since connection are getting open even after download i.e. during validation and installation phase when internet connection shouldn’t be needed anymore. These issues made processes such as opening node (category) on review page, validation, installation… take much longer time and frustrate users with time it takes and failures at the unexpected times. They also made servers and network saturate especially for real life situation where several perhaps hundreds or thousands users are trying to access an update site. This can look like denial of service attack. Introduce Callisto and all these problems are exuberated and more apparent and more frustrating to everyone. We created patch that will resolve these problems. This patch introduces two things to update. First, inclusion of data about included features in the digest which takes care of premature and perhaps unnecessary downloading of feature jars and greatly improves user browsing experience and somewhat lowers bandwidth needed. Second, introduces concept of session to update. This second feature greatly lowers bandwidth requirements and possibility of server, network and client throttling down.
Created attachment 43244 [details] update core and ui patch
Created attachment 43245 [details] apache log before patch is applied
Created attachment 43246 [details] apache log after patch is applied
Notice number of lines/gets in logs as well as time it took (using timestamps) with and without patch. I will post more detailed explanation and analysis of logs soon.
I counted 1136 lines in the log before patch, 254 after :-).
McQ, Jeff, this is the bug we discussed during the Eclipse call. It addresses the two most pressing problems we have identified while using Update against the Callisto web site: 1) It adds included features into the digest (included features cause major user experience problems because they are trip-loaded while manipulating the UI, namely during validation and prereq closure; we painfully realized that the Callisto site resembles an iceberg - most of the features are 'under the surface' :-). 2) It adds a very simple URL cache that is maintained during one install session. This dramatically cuts on the number of connection attempts. We have learned that connection requests can represent most of the total resource download time for small files, which means that checking for resource timestamps takes almost as long as downloading resources outright. In addition, checking the timestamps during verification stage does not even make sense since the resources have already been downloaded. As you will see, the attached patch is not as small as we would collectively like at this stage of the project. However, we do beleive that in this instance, accepting the risk makes sense. Note also that we have a built-in workaround for unexpected problems with digests - all that is needed is to remove the digest from the site and the Update Manager will revert to the old code paths. True, that will make site browsing slower, but the function will still be there and working.
The problem with any cache is consistency. How confident are you that there are no places where you could be getting new *content* from the server in response to decisions based on old timestamps? Basically, at the point where you do decide you need to get content, you must always validate the cache.
We obviously want to get the new content from server as soon as it made available. What we have done here is acknowledge that checking the timestamp before an install operation is enough. We don't need to frantically check several times throughout the operation (as it has been done before). What does it mean to check anyway? We made certain decisions regarding features, dependencies, plug-in complement etc. during the install process, so refreshing the site halfway would just create an inconsistent situation. The state we are proposing here is not very aggressive. If you cancel the install and invoke it again, we will check the site timestamp again. All we want to do is to claim that one check at the beginning of the install session ought to be enough for the duration of the session.
Measurements are done with Eclipse 3.2 RC6, Java Hotspot 1.5, Windows XP, Apache 2.2, Pentium M 1.6GHz, 2GB of RAM. Apache and Eclipse were co-located on the same machine (my laptop). Logs analysis and explanation: Getting to the review page: Without patch: 23:02:12 - 23:02:13 5 connections With patch: 22:41:26 - 22:41:26 2 connections Browsing the site. Selecting different features, pushing “Select Required” button and finally accepting downloading: Without patch: 23:03:11 - 23:03:31 84 connections With patch: 22:41:27 - 22:52:03 0 connections Downloading features and plug-ins: Without patch: 23:05:19 - 23:05:24 297 connections With patch: 22:52:04 - 22:52:10 252 connections Validation and Installation: Without patch: 23:10:23 - 08:26:22 750 connections With patch: 22:52:11 - 22:52:42 0 connections Timestamps are given here not to measure time (although Validation and Installation stage took a lot longer without patch precisely because of number connection i.e. saturation of TCP/IP), but to show where in the logs each stage happens. For the run with the patch timestamps for browsing site, and validation and installation, stages do not exist in the log since apache was not contacted. I put them here to show when it was happening, so that it can be compared with the run without patch. Browsing site with the patch was smooth without any jitters or waiting times for something to happen. On the other hand without the patch was slow and annoying… as we all came to know it.
Dejan is right. Plus there are additional things to consider about how update works right now. Let’s say you picked your features/plugins and downloaded them. Now you have everything you need to proceed with installation locally. However, since we will check for this files on the server again installation will fail if somebody has removed any of those features from the site or if the site is not reachable anymore. That is not consistent with anything since if you can get broken by data you do not need. There are two reasons that we are not worried about stale information: 1) in current update setup the only time it will be detected and make sense is when “real” download starts and the same will happen with new session 2) what Dejan said in above comment. Even if person somehow works with stale data during browsing of the site when s/he gets to download stage he will be stopped and s/he will have to restart the whole process (this could easily happen with current update too). However kicker here is getting to the download stage now is very fast (as shown by the above analysis), so this is not bad proposition as before.
+1
+1 !! This is not only good for server bandwidth and client wait times, but also for server RAM -- Apache connections are costly on the server's RAM so this change is huge. IMHO caching the data and compating it to response.getLastModified() only makes sense from a sysadmin's point-of-view. Thanks for cc'ing me on this. D.
thanks McQ, thanks Denis.
Soliciting more votes - Wassim?
Should we split this patch/bug report into 2 separate pieces...one for the digest change and one for the session change?
We can provide two patches that address the two problems individually, but we really cannot make Sophia's choice on which one is more important - we want them both in.
I think its always been the case that we try to enter separate bug reports for all problems in order to easier track them. I think this is increasingly important with these 2 problems considering this is the day before the final build of the release cycle and the patch is touching 14 separate Java files. Personally I would feel more comfortable reviewing the patches separately so I don't get confused as to what code is fixing what bug. For instance, currently I see a change to ContentReference#toString and I cannot tell if it is necessary (does this make it an API change?) or just gratitous.
DJ, although this is possible they really go hand in hand. But, to explain results benefits from digest are seen in "browsing site" stage(caching here removes only 3-4 unnecessary connections), the rest of the stages benefits are from url caching. Patch split is like this: Digest: src/org/eclipse/update/internal/core/SiteFile.java src/org/eclipse/update/internal/core/ExtendedSite.java src/org/eclipse/update/internal/provisional/SiteOptimizerApplication.java src/org/eclipse/update/internal/operations/UpdateUtils.java src/org/eclipse/update/internal/operations/OperationValidator.java src/org/eclipse/update/internal/ui/wizards/ReviewPage.java Cache: src/org/eclipse/update/internal/core/FeaturePackagedContentProvider.java src/org/eclipse/update/internal/core/UpdateManagerUtils.java src/org/eclipse/update/internal/core/InternalSiteManager.java src/org/eclipse/update/internal/core/UpdateCore.java src/org/eclipse/update/core/FeatureContentProvider.java src/org/eclipse/update/core/ContentReference.java src/org/eclipse/update/internal/core/UpdateSession.java src/org/eclipse/update/internal/ui/wizards/InstallWizard.java
Changes to ContentReference#toString are necessary because id is not always set before toString is used, so there are cases that same file would be downloaded twice just because it would be cached once as url() and once as url(pluginID).
I'm seeing an NPE when running with this code. To reproduce: - install all of callisto - restart - go back to the update site, - select WTP Updates
oops not done... - click finish - on the result page browse the content --> NPE This is not really reinsuring. I will attach the stack trace
Stack trace about the previous NPE: java.lang.NullPointerException at org.eclipse.update.internal.core.ExtendedSite.getLiteFeature(ExtendedSite.java:70) at org.eclipse.update.internal.operations.UpdateUtils.getIncludedFeature(UpdateUtils.java:675) at org.eclipse.update.internal.operations.OperationValidator.computeFeatureSubtree(OperationValidator.java:522) at org.eclipse.update.internal.ui.wizards.ReviewPage.isFeatureProblematic(ReviewPage.java:1637) at org.eclipse.update.internal.ui.wizards.ReviewPage.access$4(ReviewPage.java:1621) at org.eclipse.update.internal.ui.wizards.ReviewPage$TreeLabelProvider.getImage(ReviewPage.java:266) at org.eclipse.jface.viewers.StructuredViewer.buildLabel(StructuredViewer.java:2103) at org.eclipse.jface.viewers.TreeViewer.doUpdateItem(TreeViewer.java:253) at org.eclipse.jface.viewers.AbstractTreeViewer$UpdateItemSafeRunnable.run(AbstractTreeViewer.java:95) at org.eclipse.core.runtime.SafeRunner.run(SafeRunner.java:37) at org.eclipse.core.runtime.Platform.run(Platform.java:843)
it seems that callisto has some other problems today. Several people reported this to me. Plus callisto needs new digest to use this patch.
The other things that I'm worrying about are: - what happens when someone with rc6 goes to an update site containing the new format of digest?
The callisto site that I'm connecting to is working fine when I'm running without the patches.
Here are some comments on the patch: - ContentReference.toString() - Being so dependent on the format of toString() feels dangerous - by convention this is just a string used for debugging information. - ExtendedSite.setLiteFeatures - if an empty array parameter is input to this method, the liteFeatures field will not be updated. In this case allLiteFeatures and liteFeatures will have inconsistent values - If a null parameter is provided, then this method will have an NPE. And, a null parameter is definitely possible because of this line in DefaultSiteParser: extendedSite.setLiteFeatures(getLightFeatures(extendedSite)); because getLightFeatures can return null. - ExtendedSite.getLiteFeature(VersionedIdentifier vid) - There is no null check on the allLiteFeatures field, but it seems possible that it has not been assigned yet. - SiteOptimizerApplication.runDigestBuilder - The for loop iterates up to featureList.size(), but the featureList variable can be replaced within the loop. It's generally dangerous to be replacing the list from within the iteration block. Especially so since the list may be set to null, which would cause a NPE on the next iteration of the loop. - InstallWizard.performFinish - Shouldn't it reset the session state in all cases? It looks like if the user cancels the session state will not be cleared.
Pascal, there are four scenarios we have identified: 1) RC6 used to access an update site with a digest built using the new format (forward compatibility) - Everything works as expected (RC6 ignores included features in the digest) 2) RC6 + patches used to access an update site with a digest built using RC6 (backward compatibility) - Everything works as expected (the new code works with the features found in the digest). 3) RC6 + patches used to access an update site with a digest link in site.xml but no digest on the site - NPE. This is the case Pascal and JohnA found and is related to some mirrors that occasionally fail to copy the digest while they still declare it in the site.xml. Although this is technically a an update site problem and should be dealt with by the site admin, Update should not react with an NPE. We will add a check for this case and attach a replacement patch. 4) RC6 + patches used to access an update site with no digest link in site.xml ( - The code works identically to RC6 i.e. it goes through the old code paths. We will attach a replacement patch with the NPE checks.
This starts to sounds by Monty Python, but there are 5 (five) scenarios - I missed an (obvious) one where RC6+patches is used to access a site with a new digest format. I missed it because I was concentrated on the compatibility cases, not the ideal one.
ContentReference.toString() – completely agree, but this was used to key the downloaded files cache, so in order to keep the patch as small as possible I used the convention used elsewhere in Update (however imperfect it is). ExtendedSite – will provide a replacement patch with checks for null and empty array at the beginning of these methods SiteOptimizerApplication - command line utility is not used in multi threaded environment, so no concurrent updates possible. InstallWizard.performFinish - there are many other possible exit points, but only one entry point and that's the one used to reset/start the session.
Created attachment 43317 [details] patch
SiteOptimizerApplication - I wasn't referring to a multi-threaded case. The list can become null because addFeaturesToList can return null. Also, I know we generally don't support other clients using the update core APIs, but it seems if anyone does they will get stale information because UpdateSession.reset() will never be called (it's currently only called by the wizard). What is the impact of this change for anyone using the (interim) update core APIs?
John, the first issue is something I would not worry too much about, although we can address it if you feel it is needed. The site enhancer application is a site admin utility and in the rare case where it chokes on missing features, it is easy to fix the problem without involving actual users. The second problem is real. You are right that the (rare) users of Update Core APIs may find themselves with stale site and feature objects if the site changes while the application is up. The cleanest solution would be to hunt all the possible exit points of the install operation and call 'reset' there, which an error-prone job now but can be attempted for 3.2.1. What worries me is that the majority of Update users will access Update through the GUI and they will find Update mostly unusable with all but the best of connections and servers. Callisto simply pushed Update beyond its comfort level and we must make the hard choices now or give up the idea to serve Callisto this way. My suggestion is as follows: 1) Apply the patch now (RC7) and move Callisto to the latest digest format 2) Use the time between RC7 and 3.2 to gather feedback from people who create site and feature objects programmatically 3) Work with these users to see if they can call 'UpdateCore.getUpdateSession().reset()'. I know this is internal but it will get them going until 3.2.1. 4) Hunt all the exit points and call 'reset()' ourselves in 3.2.1 time frame. I think it is a tradeoff that is worth making.
+1 I talked with Dejan and he convinced me of the importance of fixing this. It's clearly a big risk to release a significant change this late, but it seems necessary if there is any hope of making Callisto update usable.
If this goes in and we are ready to risks, then I would suggest that we also consider releasing a real one liner from bug #141887 which is also very important.
released.
WOW, what a difference! Branko and I worked to created the "new and improved" digest.zip file for the callisto staging area. This staging area is currently identical to the callisto releases discovery site (which, if you've lost track, now has Callisto RC4 features in it). The only difference as of this evening, on June 1, is the digest.zip file. Give it a try, by defining a new "remote site" to update manager, with http://download.eclipse.org/callisto/staging Of course, you'll need to use an RC7 base eclipse to get the benefit. I got a "sneak preview" with some patches from Branko, and I must say, the update manager is actually starting to get usable! In particular, the initial validation and "select required" action is instantaneous, instead of a 30 second or so delay. (on my home line). I believe this change alone, if no other, is sufficient to suggest an RC4a Callisto, with no changes except an RC7 base update. If I hear no complaint here in this bug, I will suggest this on cross-project list, and suggest a roll-out date of 6/9. That would give projects enough time to do some modest amount of testing/use of RC7, to get some confidence there's nothing that breaks, if we started to encourage users to use RC7 with no upstream projects updating anything. Nice job Branko, and others contributing to this effort (by your careful reviews, etc).
I have just installed staging Callisto into the RC7 build from from home. The browsing was very fast and the install went smoothly (I was asked to retry but it has more to do with my flaky Internet connection today than anything else). The install phase was fast with no network traffic. This looks pretty good from my end.
Created attachment 43356 [details] core patch After talking to John and Dejan this morning we decided that url caching should not start automatically. So we made this patch that will disable url caching until UpdateSession.reset() is called. This does not affect regular Update UI paths and will leave stuff at RC6 level for anyone who is not aware of url caching i.e. old code that is using Update core will function like in RC6. This is done to elevate some of the issues mentioned in comment 32. This is a patch on todays head.
This enhancement makes this patch virtually regression-free. Users who access Update APIs will not see any change in behaviour. When Install wizard enables cashing, it will eventually prompt users to restart Eclipse, and most users do since it is recommended. Restarting eclipse will revert things to normal.
Are you looking for another round of confirmations?
I would claim that this is still the same patch, just a tweak to resolve the final issue raised by John (and provided by him - thanks, John!).
understood. I wasn't complaining, just checking if you needed me to +1.
We sent a Go for RC7.
Fixed.
I tested now with RC7 and Callisto RC4 staging area. There is only one remaining issue: after selecting all Callisto projects, accepting the license, and clicking finish, the UI becomes completelly frozen for about 3 minutes. After that, the download starts.
Hi guys, I concur with the last comment. I just downloaded a fresh RC7 and tried it against the Callisto RC4 staging site. The Update Manager was much much quicker until I clicked Finish - then my workbench froze for several minutes (wouldn't even repaint - WinXP) before starting downloads.
I suggset a new bug be open for comment 46 and comment 47, and the originators should specify which VM's are being used (not sure that's releated, but, might be). Thanks.
By the way, I am seeing that only opening "Help - > Find and Install..." wizard takes from 30 to 60 seconds. I do have number of plugins, including WTP, Mylar, Subclipse, Spring IDE (split across 5 or six locations) and over 40 bookmarks registered. But still it should not take that long because workbench UI is blocked all that time.
(In reply to comment #48) > I suggset a new bug be open for comment 46 and comment 47, and the originators > should specify which VM's are being used (not sure that's releated, but, might > be). Done. bug#145247
*** Bug 68442 has been marked as a duplicate of this bug. ***
*** Bug 140555 has been marked as a duplicate of this bug. ***
*** Bug 94035 has been marked as a duplicate of this bug. ***
*** Bug 83907 has been marked as a duplicate of this bug. ***
*** Bug 83741 has been marked as a duplicate of this bug. ***
*** Bug 63932 has been marked as a duplicate of this bug. ***
*** Bug 105714 has been marked as a duplicate of this bug. ***
*** Bug 107211 has been marked as a duplicate of this bug. ***
*** Bug 140391 has been marked as a duplicate of this bug. ***
*** Bug 127290 has been marked as a duplicate of this bug. ***
*** Bug 140371 has been marked as a duplicate of this bug. ***
*** Bug 79212 has been marked as a duplicate of this bug. ***
*** Bug 84277 has been marked as a duplicate of this bug. ***
