weixin_39517520
weixin_39517520
2020-12-28 21:57

CRM-21298 System check: bypass missing indices check until update indices works

Overview

The "Performance warning: Missing indices" system check should be bypassed until the "Update Indices" task works reliably.

This is in the RC branch because the Update Indices problem is not resolved in 4.7.26

Before

Users of many sites (including as one example a demo site where 4.7.17 was installed and then upgraded to 4.7.25) are presented with a WARNING level message in the system check stating that they have missing indices. An Update Indices button is in the message. Clicking the button (for many sites) causes a database error as documented in CRM-20817.

After

The system check will never produce the "Performance warning: Missing indices" message.

Technical Details

I left all the code in there to run the check and update the indices. The checkIndices() function just returns an empty array before getting to that point. This allows for easy removal for testing and when the update tool is actually reliable.

该提问来源于开源项目:civicrm/civicrm-core

  • 点赞
  • 写回答
  • 关注问题
  • 收藏
  • 复制链接分享
  • 邀请回答

9条回答

  • weixin_39851974 weixin_39851974 4月前

    I get comment but I wouldn't personally see this as meeting the bar for an rc merge because it is not a recent regression or newly identified critical issue

    点赞 评论 复制链接分享
  • weixin_39517520 weixin_39517520 4月前

    I agree with the principle that has laid out. My intent in doing this was not to attempt to fix the problems with Update Indices but to quit pushing people to try it.

    I have no stake in this, though--in fact, the bug has been good for Civi911 business as people call and fixes things manually. I just think it's a bit embarrassing for the product.

    点赞 评论 复制链接分享
  • weixin_39517520 weixin_39517520 4月前

    i don't know what conclusion you and have gotten from this discussion, but it would be good to get a decision so at the very least this can get in 4.7.28 and we don't have yet another release popping up messages telling people to run a feature that is broken.

    点赞 评论 复制链接分享
  • weixin_39517520 weixin_39517520 4月前

    I know everyone's got a lot on their plates, but 4.7.27 was released and 4.7.28-rc was cut without ever letting me know this PR was going to be rejected. Would it be okay to open this against 4.7.28-rc, or would you rather this be proposed against 4.7.29.

    I hate to be a nag, but I really think the situation of telling people to run a process that crashes all the time is terribly embarrassing for the project.

    点赞 评论 复制链接分享
  • weixin_39851974 weixin_39851974 4月前

    Hi my point above was that in general I don't think this is a patch for an rc - it should go against master. Rc patches should only relate to recent regressions (or sometimes improvements to fixes already in the rc) and critical bug fixes

    点赞 评论 复制链接分享
  • weixin_39517520 weixin_39517520 4月前

    I agree with the general principle --I had just thought this was a critical bug fix. (Or at least a way of papering over the critical bug until it's actually fixed.) It's all moot anyway because this PR is against a RC branch for a release that's already out.

    The problem here is that nobody made a decision to merge or close this, and now I need to know whether to submit this for 4.7.28-rc or master.

    点赞 评论 复制链接分享
  • weixin_39760433 weixin_39760433 4月前

    Replaced by #11250.

    点赞 评论 复制链接分享
  • weixin_39954908 weixin_39954908 4月前

    Tim asked me to help with this PR by clarifying some of its context and looping in others who have worked on the relevant code.

    From what I can tell, this change is bypassing earlier changes made by and merged by in #10908. Do either of you have thoughts about this PR, especially since it is made against the RC branch and thus requires action more quickly than other PRs?

    点赞 评论 复制链接分享
  • weixin_39517520 weixin_39517520 4月前

    Thanks for taking a look at it. The fact that 4.7.25 included #10908 was the only reason I even tried "Update Indices" because I knew that it failed so frequently before. Sadly, it still caused the DB error on 4.7.25. Seeing that CRM-20533 and CRM-20817 are still open, it seems like others are still seeing related problems.

    The problem here is that unlike most bugs in CiviCRM, this one has a message that pops up and directs you to the broken feature. The purpose of this PR is to suppress that message temporarily until CRM-20533 and CRM-20817 are closed.

    点赞 评论 复制链接分享

相关推荐