search

openfrontier / docker-gerrit

Build a Docker image with the Gerrit code review system
Apache License 2.0
196 stars 118 forks source link

Limit offline indexing to "accounts" index #46

Closed crorvick closed 7 years ago

crorvick commented 7 years ago

Regenerating the index can delay startup substantially for large repositories, and doing so is unnecessary since version 2.11.2 when online indexing was added.

crorvick commented 7 years ago

Any thoughts on this one? Currently it takes about 20 minutes for the container to come up with a repo containing about 16,000 changes. If you have a suggestion for a workaround I'd love to hear. Or maybe this step could be conditional on something in the environment?

thinkernel commented 7 years ago

Hi @crorvick . Sorry for the late response. The re-index lines that you want to remove were added in c78e429 which is a commit that I want to make sure a smooth upgrading from the Gerrit 2.12.x to 2.13.x. People can just remove their 2.12 container but keep the volume then run a new 2.13 container with the same volume. The starting process will upgrade the repository for them smoothly.

However, since 2.13 the Gerrit introduces a new secondary index for accounts, and this must be indexed offline before starting Gerrit. And I chose rebuilding all indexes instead of just the accounts because there's an issue shows when you upgrade to 2.13 or 2.13.1 you'll have to rebuild both indexes offline and, more importantly, there's a risk that some day online reindexing might not works again. Clearly, I didn't aware of the effect to restart a Gerrit installation with many changes like yours. I have to agree with you that 20 minutes is unacceptable for just a restarting.

Here's my proposal. Would you mind changing the re-index all offline logic into the just re-index the accounts offline? If the re-index time for accounts is acceptable, we can speed up the restarting process while keep the upgrading process the same way as it is.

crorvick commented 7 years ago

Thanks for the detailed explanation. That makes sense, and your proposed fix worked well. I'll push a new commit shortly.

thinkernel commented 7 years ago

This is merged and tested. The upgrading process looks good. You can pull the latest image and test it on your project, now. Great works!