Make WordPress Core

Opened 17 years ago

Closed 15 years ago

Last modified 3 months ago

#10638 closed defect (bug) (worksforme)

Wordpress would randomly fail, because of server configuration

Reported by: kiprasm's profile kiprasm Owned by:
Milestone: Priority: normal
Severity: critical Version: 2.8.1
Component: General Keywords: implicit_flush random failures has-patch
Focuses: Cc:

Description

Hey,
this is a bug report + plus an actual way to fix it.

Basically, i just had an entire half of day of headaches, of why wordpress fails randomly. Sometimes it wont save pages, sometimes the voting plugin won't work, sometimes blog activation or image uploading would fail etc. etc.

Turns out it is my server configuration, which did not implicitly flush the output buffer after a script has finished executing (php.ini implicit_flush was set to 'Off'). Because of that, some ajax scripts would not work, wp-admin/index.php would freeze (left sidebar menu items would not expand after clicking on the arrows on the right), activation script was broken (only the top half would be shown), sometimes redirects would fail, throwing just blank pages (after a post edit for example), i could go on and on.. And all of this was fixed very easily (i am actually very surprised, has it never happened to other people??).

To fix it, i've added these 2 lines to wp-config.php:

ini_set('implicit_flush', 'On');
ob_implicit_flush();

Works like a charm now. I really think you should consider adding these lines to the default Wordpress package as well.

Change History (13)

#1 @mrmist
17 years ago

A number of sites work for me with implicit_flush off in the PHP settings, so this may be something of a red herring.

#2 follow-up: @filosofo
17 years ago

Also, WordPress has a callback wp_ob_end_flush_all() attached to shutdown, so everything should already be getting flushed.

#3 in reply to: ↑ 2 @kiprasm
17 years ago

Replying to filosofo:

Also, WordPress has a callback wp_ob_end_flush_all() attached to shutdown, so everything should already be getting flushed.

I've tried to put ob_flush() and flush() and them both manually at the end of scripts, and that wouldn't help. That is probably why wp_ob_end_flush_all() didn't work either, as i see it uses ob_end_flush(). The one other thing that worked for me tho, is this sequence:
ob_end_flush(); ob_flush(); flush(); ob_start();
When i put that at the end of wp scripts, it would also work, and this was my first working solution, which i later replaced with the two lines in wp-config.php, when i saw that they work too.

Also, it might be that something is wrong with my server, since as you said a number of sites work with this configuration. Still this way it works for me, might also work for some other people out there..

#4 @ryan
17 years ago

  • Milestone changed from 2.9 to Future Release

#5 @dd32
15 years ago

  • Milestone Future Release deleted
  • Resolution set to worksforme
  • Status changed from new to closed

As it seems it's a specific server environment which does not play well with outputting 100% of page requests....

This ticket was mentioned in PR #11064 on WordPress/wordpress-develop by @dmsnell.


3 months ago
#6

  • Keywords has-patch added

The build change in #10638 removed many required files and added them to .gitignore. This led to WordPress crashing when loading wp-load.php until the nom run build:dev command was run.

Deleting these files had a number of deleterious side-effects:

  • They no longer appear in wordpress-develop, making it hard to see what code is going to run and hard to reference the files in places like Github where git CLI tools and the Gutenberg repo are not colocated and handy.
  • Any version-history for the files is lost, making it particularly difficult to examine how those files changed with the Gutenberg sync and update cycle, or how to track what ran at what version of WordPress.
  • Because of the missing history, this severely impacts the ability to debug issues or git bisect to find root cause and changes containing the root cause.
  • Any changes which are made to these files is untracked, making edits at high risk for being lost without any way to undo or revert. Previously, changes might be missed because they need to sync to Gutenberg, but after the build change, those edits never appear to warn someone that they edited the file in the wrong repo.
  • Benchmarking, debugging, and analysis flows which scan through the source control revisions now take 10x, 100x, 1000x longer to run because npm ci has to run, download more than 1 GB of npm packages, and rebuild all of the files on every git checkout. this practically eliminates the practicality of running workflows which assess the project over time.

This PR brings back those files and connects them to their pre-build-change version history by branching from a point in trunk immediately before the build change.

Although they were deleted in trunk, this patch, when applied _as a merge commit_ will provide two parents which will allow any and all git tooling to reconstruct the history of the files without any special options or flags.

## Status

This is ready for a full review.

https://github.com/user-attachments/assets/bffab022-1262-4086-ab95-f86fd918217e

---

Note:

  • This _must_ merge as a non-squashed commit, because the commit parent must be accessible to preserve history.
  • This needs to also come through in the svn-to-git conversion. If the merge is lost in that process then it won’t matter if this PR restores the history, because it will be lost when back-writing from the Subversion source.

## Testing

Here is a sequence of events that simulates this operation. The script creates an SVN repo, adds some files and makes a few meaningless commits, then deletes a test.txt file.

There is a git svn mirror tracking the SVN repo.

From the SVN side, a new branch is created to restore the files. It is forked from before they were deleted and then the test.txt file is modified in a neutral way so that it will create a merge conflict. This is important because otherwise svn and git will automatically accept the deleted files as the truth.

That branch is merged in which tracks the version history for the files, because it maintains metadata pointing to the commits before they were deleted.

On the git svn side though it’s critical to first git svn fetch --all to retrieve the new branch (otherwise it will not have the metadata and therefore linearize the merge), and then to run git rebase --merge --rebase-merges so that it avoid linearizing the merge.

This has been confirmed and the svn branch fixes-64393-restore-version-history has already come over from svn into the git mirror — this should work as expected.

https://github.com/user-attachments/assets/ccc5cd1b-2e61-4895-ac1f-4450299bffeb

#!/usr/bin/env bash

ROOT=$(pwd)
REPO=file://${ROOT}/svn-repo

printf "\e[90mStep \e[33m1\e[90m — create SVN repo\e[m\n"
svnadmin create svn-repo
svn mkdir ${REPO}/trunk -m "Create trunk"
svn mkdir ${REPO}/branches -m "Create branches"

printf "\e[90mStep \e[33m2\e[90m — load content into SVN checkout\e[m\n"
svn co ${REPO} svn-checkout
pushd svn-checkout/trunk
svn up ..
echo "Testing instructions" > test.txt
echo "1 2 3 4" > data.dat
svn add test.txt data.dat
svn commit -m "Initial commit"
popd

printf "\e[90mStep \e[33m3\e[90m — establish \e[2mgit\e[0;90m mirror\e[m\n"
git svn clone -T trunk -b branches ${REPO} git-repo

printf "\e[90mStep \e[33m4\e[90m — make noisy change\e[m\n"
pushd svn-checkout/trunk
echo "\n\n1. look at it" >> test.txt
echo " 5" >> data.dat
svn commit -m 'Expand testing'

echo "<?php\n\ndie();" > feature.php
svn add feature.php
svn commit -m 'Add feature skeleton'
svn up
popd

pushd git-repo
git svn rebase 
popd

printf "\e[90mStep \e[33m5\e[90m — remove test.txt\e[m\n"
pushd svn-checkout/trunk
svn rm test.txt
echo "/test.txt" > .gitignore
svn add .gitignore
svn commit -m 'Remote tests'
popd

printf "\e[90mStep \e[33m6\e[90m — make more noisy commits\e[m\n"
pushd svn-checkout/trunk
echo "<?php\n\ndie(1);" > feature.php
svn commit -m 'Die abnormally'

echo "Are you lost?" > directions.html
svn add directions.html
svn commit -m 'Add directions'
svn up
popd

pushd git-repo
git svn rebase 
printf "\n\n\e[90mState of the git repo after deleting the tests\e[m\n"
git log --graph --topo-order --oneline
popd

printf "\e[90mStep \e[33m7\e[90m — Branch before removal to restore\e[m\n"
pushd svn-checkout/trunk
svn copy -r5 ${REPO}/trunk ${REPO}/branches/restore-tests -m 'Restore tests (branch from f5 before removal)'
svn up ../
popd

pushd svn-checkout/branches/restore-tests
echo "\n" >> test.txt
echo "" > .gitignore
svn commit -m 'Introduce merge conflict to force restoration'
svn up ../
popd

pushd git-repo
git svn rebase 
popd

printf "\e[90mStep \e[33m8\e[90m — Noisy commit\e[m\n"
pushd svn-checkout/trunk
echo "<?php\n\ndie(0);" > feature.php
svn commit -m 'Die normally'
svn up
popd

pushd git-repo
git svn rebase 
printf "\n\n\e[90mState of the git repo after noisy commits\e[m\n"
git log --graph --topo-order --oneline
popd

printf "\e[90mStep \e[33m9\e[90m — Merge branch and restore tests\e[m\n"
pushd svn-checkout/trunk
svn merge ${REPO}/branches/restore-tests --accept postpone
ls ../branches/restore-tests
cp ../branches/restore-tests/test.txt .
svn resolve --accept working test.txt
svn add test.txt
svn commit -m 'Merge and restore tests'
svn up ../
popd

pushd git-repo
git svn fetch --all
git svn rebase
printf "\n\n\e[90mState of the git repo after the merge\e[m\n"
git log --graph --topo-order --oneline
popd

@dmsnell commented on PR #11064:


3 months ago
#7

Changed branch HEAD from 29b508c2feaba4a305ec8c969395b923e16827ef to 4220ecc44fd76b405ca4f39092f32a439f9a0230

This is to explore a new process to rebuild the branch. Details to come later, but I’m going to try and merge trunk into the branch at each sync-commit along the way.

@dmsnell commented on PR #11064:


3 months ago
#8

Recreated branch from previous 59e0a279175edddf034304821f0430e3671d62ad to new 49093f1b4428b6bf0d4f6bfdc97a4135207bc4b1

This time the creation was automated with the following script:

https://gist.github.com/dmsnell/c6f068e7f6028bacbfcb810d2fe726c2

@jonsurrell commented on PR #11064:


3 months ago
#9

I picked a file that I expected to have a long history. The paragraph block.json. If I start at the HEAD of this branch and navigate to its earliest history, I reach https://github.com/dmsnell/wordpress-develop/commit/f7d617cfb8e71ead2ab589011bffad5a22f35d3c. If I start at an arbitrary place in Trac before the break in history (6.0) and navigate back, it traverses merges and ultimately arrives at the corresponding commit: https://core.trac.wordpress.org/browser/trunk/src/wp-includes/blocks/paragraph/block.json?rev=48262

That file doesn't exist at all in `trunk` right now.

It's a good indicator that this branch is an adequate solution to fixing history. 👍

---

I'm certainly missing some background, but I'd like to understand how this will work in practice.

Although they were deleted in trunk, this patch, when applied as a merge commit will provide two parents which will allow any and all git tooling to reconstruct the history of the files without any special options or flags.

Ultimately, things need to happen in subversion and then propagate to git mirrors from there. Work is committed with svn commit. Will this rely on git svn dcommit or something along those lines? Will a branch need to live on subversion permanently to hold these commits as a bridge from the current trunk revision back to the history-restoring point?

@dmsnell commented on PR #11064:


3 months ago
#10

Ultimately, things need to happen in subversion and then propagate to git mirrors from there. Work is committed with svn commit. Will this rely on git svn dcommit or something along those lines? Will a branch need to live on subversion permanently to hold these commits as a bridge from the current trunk revision back to the history-restoring point?

Thanks @sirreal for your review and the question. This is something I wanted to ensure was simulated before merging, which you can see if you use the attached simulator script.

To the best of my knowledge, when subversion creates the merge with svn merge, then git svn rebase will create a merge commit, as long as the commits from the branch have been previously fetched with git svn fetch. We already have the branch created on the git side, so I am confident that as we commit the changes into the svn branch, they will carry over into git as well.

The final commit will be the merge commit itself, and since both of its parent’s will already by synced, we should have no problem seeing it created in git

My plan was to recreate the commits in svn first rather than using git svn dcommit but I believe we could use that approach too.

@dmsnell commented on PR #11064:


3 months ago
#11

Recreated branch from 45aafcc1882b70349839aee9e244c0ca5afda312 to af434d639d93f2c3eeecbee9e8d164d92111326b

@jonsurrell commented on PR #11064:


3 months ago
#12

The commits here in GitHub seem to be correct and will likely resolve the issue of broken and lost history that this seeks to address.

This isn't a simple commit, much of the interesting work here is the process of how the commits are actually made. I've reviewed the process with @dmsnell and it seems to work well.

@dmsnell commented on PR #11064:


3 months ago
#13

Merged in [62143]
08be2764e3f46f2729e5a4f0af34a6d3150d640b

Note: See TracTickets for help on using tickets.