New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pagespeed lost img tag src value,when pagespeed is too busy to rewrite image #707

Closed
GoogleCodeExporter opened this Issue Apr 6, 2015 · 10 comments

Comments

Projects
None yet
1 participant
@GoogleCodeExporter
Copy link

GoogleCodeExporter commented Apr 6, 2015

What steps will reproduce the problem?
1.Use apache as forword proxy
2.set ModPagespeedImageMaxRewritesAtOnce      1
3.Test pagespeed's performance with ab or loadrunner until pagespeed is Too 
busy to rewrite image.
If you want to reappear the issue quickly, you can modify the source code as 
below and rebuild.
RewriteResult ImageRewriteFilter::RewriteLoadedResourceImpl(
      Context* rewrite_context, const ResourcePtr& input_resource,
      const OutputResourcePtr& result) {
         if (work_bound_->TryToWork()) {
         }
      }
TO:
RewriteResult ImageRewriteFilter::RewriteLoadedResourceImpl(
      Context* rewrite_context, const ResourcePtr& input_resource,
      const OutputResourcePtr& result) {
         if (0) {
         }
      }   

What is the expected output? What do you see instead?
The orginal info: 
<img width="320" height="143" 
src="http://r3.sinaimg.cn/2/2013/0524/a0/9/03424793/320x143x100x0x0x0.jpg">

Expected:
<img width="320" height="143" 
src="http://r3.sinaimg.cn/2/2013/0524/a0/9/03424793/320x143w320x143x100x0x0x0.jp
g.acce.ic.5df2CTUCSE.webp">

Instead:
<img width="320" height="143" src="">

What version of the product are you using (please check X-Mod-Pagespeed
header)?

1.1.23.2-0

On what operating system?
SUSE

Which version of Apache?
Server version: Apache/2.4.3 (Unix)

Which MPM?
Server MPM:     event
URL of broken page:
http://3g.sina.com.cn/



Original issue reported on code.google.com by yanzhenl...@gmail.com on 24 May 2013 at 9:09

@GoogleCodeExporter

This comment has been minimized.

Copy link

GoogleCodeExporter commented Apr 6, 2015

I have tried to modify the original soruce code 
if (work_bound_->TryToWork()) {
    //ignore
  } else {
    image_rewrites_dropped_due_to_load_->IncBy(1);
    message_handler->Message(kInfo, "%s: Too busy to rewrite image.",
                             input_resource->url().c_str());
  }

to:

if (work_bound_->TryToWork()) {
    //ignore
  } else {
    CachedResult* cached = result->EnsureCachedResultCreated();
    cached->set_url(input_resource->url().c_str());
    image_rewrites_dropped_due_to_load_->IncBy(1);
    message_handler->Message(kInfo, "%s: Too busy to rewrite image.",
                             input_resource->url().c_str());
  }

and test ok,but i am not sure whether it's the best implementation or not.

Original comment by yanzhenl...@gmail.com on 24 May 2013 at 9:35

@GoogleCodeExporter

This comment has been minimized.

Copy link

GoogleCodeExporter commented Apr 6, 2015

[deleted comment]
@GoogleCodeExporter

This comment has been minimized.

Copy link

GoogleCodeExporter commented Apr 6, 2015

Issue 706 has been merged into this issue.

Original comment by sligocki@google.com on 24 May 2013 at 5:51

@GoogleCodeExporter

This comment has been minimized.

Copy link

GoogleCodeExporter commented Apr 6, 2015

Yikes, reproduced this locally, looking into it.

Original comment by sligocki@google.com on 24 May 2013 at 10:48

  • Changed state: Accepted
  • Added labels: Priority-High
  • Removed labels: Priority-Medium
@GoogleCodeExporter

This comment has been minimized.

Copy link

GoogleCodeExporter commented Apr 6, 2015

Original comment by sligocki@google.com on 28 May 2013 at 8:30

@GoogleCodeExporter

This comment has been minimized.

Copy link

GoogleCodeExporter commented Apr 6, 2015

Issue 598 has been merged into this issue.

Original comment by jmaes...@google.com on 30 May 2013 at 9:31

@GoogleCodeExporter

This comment has been minimized.

Copy link

GoogleCodeExporter commented Apr 6, 2015

Yes! This is the issue that I've been experiencing but couldn't figure out 
exactly why.

Original comment by mwhe...@360psg.com on 31 May 2013 at 8:22

@GoogleCodeExporter

This comment has been minimized.

Copy link

GoogleCodeExporter commented Apr 6, 2015

This has been fixed in trunk.  The problem was that rewrites that were 
abandoned due to load still had their URLs rewritten.

Original comment by jkar...@google.com on 10 Jun 2013 at 4:07

  • Changed state: Fixed
  • Added labels: Milestone-v27, release-note
@GoogleCodeExporter

This comment has been minimized.

Copy link

GoogleCodeExporter commented Apr 6, 2015

This has been fixed in trunk.  The problem was that rewrites that were 
abandoned due to load still had their URLs rewritten.

Original comment by jkar...@google.com on 10 Jun 2013 at 4:07

@GoogleCodeExporter

This comment has been minimized.

Copy link

GoogleCodeExporter commented Apr 6, 2015

Original comment by jkar...@google.com on 13 Jun 2013 at 1:33

  • Added labels: Milestone-v29
  • Removed labels: Milestone-v27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment