Skip to content
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

Linking issue dependencies fails (API returns 500 error) #19368

Closed
fnetX opened this issue Apr 10, 2022 · 34 comments · Fixed by #19475
Closed

Linking issue dependencies fails (API returns 500 error) #19368

fnetX opened this issue Apr 10, 2022 · 34 comments · Fixed by #19475
Assignees
Labels
performance/speed performance issues with slow downs type/bug

Comments

@fnetX
Copy link
Contributor

fnetX commented Apr 10, 2022

Description

Okay, we're really lost in debugging this for hours now on Codeberg. A live meeting with 5 people cannot figure out what's exactly going on. Let's start from the beginning:

On Codeberg.org, you can no longer link issue dependencies. The API call returns an error 500 with the error message in the gitea.log:

2022/04/10 18:38:42 ...api/v1/repo/issue.go:276:SearchIssues() [E] CountIssues: unable to get one row result when CountIssues, row count=0

Now of course we thought of a recent regression and reverted to pre-1.16.5: still happening. We tried to reproduce on try.gitea.io and on codeberg-test: not happening.

We tried moving a whole database snapshot into our air gapped staging instance: Not happening, not reproducible, everything fine it seems (debugging there is a bit complicated, but we tried

"https://codeberg.org/api/v1/repos/issues/search?q=smtp&priority_repo_id=453&type=all&_=164961674595"

and got many results, whereas on Codeberg we still get 500.

Accepting that we can probably only approach the issue on our real production server, @Gusted worked out some patches for us to get more information.

Printing the generated SQL query for the function by XORM, and executing this ourselves in MySQL, worked fine and returned the correct information (about 9000 in our case).

We then checked the length of the return value of models.Issues here to see if it's an issue with countIssues or if it's an underlying issue of the options query. And the return value here was zero (0), so it indicates an issue with the query options that was being passed down.

This indicates that the query is correct, but when XORM is executing it, it will not return anything.
So this seems to be a scaling issue to us, maybe because this passes the id's of all the public repos + the further issues a user has access to, which is, 17000+ on our instance. (Maybe a point for further optimization, e.g. extend the query to allow all repos with is_private = false + explicitly private repos a user has access to?)

Also very relevant according to blame: #19244, and especially this comment, as we can't explain why this would ever return something else than 1 row from the `SELECT COUNT`` call.
Please note that our tested version includes that pull request, because @6543 pushed a cherry-pick to our branch. Of course we also tried reverting it, but - still broken.

That's where we currently are. The SQL log contains many question marks, about 17k, for each repository. That's about how many question marks we see when thinking about how to further approach this. We appreciate any help. Thank you!

Gitea Version

https://codeberg.org/Codeberg/gitea/commit/99e133b593fef813188e1e6469c65c9a11ec258a

Can you reproduce the bug on the Gitea demo site?

No

Log Gist

No response

Screenshots

No response

Git Version

git version 2.30.2

Operating System

Debian GNU/Linux 11

How are you running Gitea?

custom fork / deployment, see commit ref above

Database

MySQL

@fnetX fnetX added the type/bug label Apr 10, 2022
@wxiaoguang
Copy link
Contributor

wxiaoguang commented Apr 11, 2022

I think it's still a bug, maybe caused by GROUP BY, or some aggr SQL else.
SELECT COUNT FROM ... => always 1 rows
SELECT COUNT FROM ... GROUP BY ... => maybe 0 or many rows.
The key problem is that if it is the case, then the result of the old CountIssues (row[0]) is still incorrect and need to be fiexed ......


updated by comment below: the SQL only returns one row, indeed.

@wxiaoguang
Copy link
Contributor

wxiaoguang commented Apr 11, 2022

Printing the generated SQL query for [the function by XORM](https://github.com/go-gitea/gitea/blob/ee769f7459ffba310563cd735f503d3d7576da60/models/issue.go#L1520), and executing this ourselves in MySQL, worked fine and returned the correct information (about 9000 in our case).

Can you share the SQL for CountIssues (with the patch from #19244) ?

ps: I think there might be something incorrect for the comment and the COUNT SQL. The SQL error says row count=0, then it's impossible for the real SQL from log to get 9000 rows .... can you double check that the SQL you tried is a COUNT SQL?

@fnetX
Copy link
Contributor Author

fnetX commented Apr 11, 2022

Please see http://paste.debian.net/hidden/73165403/ for the SQL query we ran (manually interpolating the values).
Please ignore the line wraps, I just added them for easier readability. The use gitea; was also added by me.

Running it results in

count
9966

so I'm sorry if I was unclear before: Of course it returned one row with the value of about 9k (or 10k actually), and not that many rows.

@wxiaoguang
Copy link
Contributor

wxiaoguang commented Apr 11, 2022

The SQL looks correct. And the result is correct too.

Since everything seems correct, I can not understand how the row count=0 failure occurs, either ........

I suspect there are something missing or inconsistent. And according to the comment "We tried moving a whole database snapshot into our air gapped staging instance: Not happening, not reproducible, everything fine it seems", there must be something different.

Another question is that is the code (CountIssues related) exactly the same between Codeberg Gitea and Official Gitea?

@fnetX
Copy link
Contributor Author

fnetX commented Apr 11, 2022

Another question is that is the code (CountIssues related) exactly the same between Codeberg Gitea and Official Gitea?

I can't see any difference. Codeberg commits are always prefixed with CB/. The blame shows that nothing is touching these functions:
https://codeberg.org/Codeberg/gitea/blame/branch/codeberg-1.16/models/issue.go

Since we can't reproduce in our staging instance with the Codeberg binary yet, we don't really have a way to hunt this down with the upstream code. We can't just deploy the plain Gitea on production I fear ...

@wxiaoguang
Copy link
Contributor

wxiaoguang commented Apr 11, 2022

I have an idea about how to debug the problem.

Maybe we can build Codeberg-Gitea with a special flag (eg: ./gitea-codeberg my-test-issue), and use the special flag to run SearchIssues / CountIssues directly with the specialized options to trigger the bug and see output. Then we just put this test binary to production server, then test with the production database without any downtime.

If the special flag can trigger the bug, then it gives us the chance to find the root case.

If the special flag can not trigger the bug either ..... it must be impossible, no reason to explain it .....

@fnetX
Copy link
Contributor Author

fnetX commented Apr 11, 2022

I like this idea, in fact I had a similar idea about trying to produce a minimal reproducible thing (e.g. a fresh binary with only the XORM call).

I think we can do this, provided that the binary does not modify any data or execute stuff (we already considered spawning a second Gitea instance, but we'd have to make sure it doesn't e.g. trigger Matrix hooks, run queues etc).

@wxiaoguang
Copy link
Contributor

wxiaoguang commented Apr 11, 2022

https://stackoverflow.com/questions/32973835/mysql-sometimes-erroneously-returns-0-for-count

And someone said that it could also be a mysql bug 😂 (although I don't want to suspect that it's mysql's problem, and that post is quite old, the described bug is not the same)

@wxiaoguang
Copy link
Contributor

I like this idea, in fact I had a similar idea about trying to produce a minimal reproducible thing (e.g. a fresh binary with only the XORM call).

I think we can do this, provided that the binary does not modify any data or execute stuff (we already considered spawning a second Gitea instance, but we'd have to make sure it doesn't e.g. trigger Matrix hooks, run queues etc).

IIRC there are some Gitea sub commands like admin or doctor, it only do some database work.

@luwol03
Copy link

luwol03 commented Apr 11, 2022

You may can also add a db user with read-only permissions to be sure nothing is changed?

@fnetX
Copy link
Contributor Author

fnetX commented Apr 11, 2022

Notable in the stackoverflow thread is that count() can return 0 when nothing matches. I wasn't aware of that, but we should keep that in mind for further debugging.

a db user with read-only permissions

Yeah, but I also feared that it would read some queue and see it would need to fire some webhooks etc. Using a subcommand that only runs the affected query appears to be safe, though (not starting the normal Gitea app itself).

@lunny
Copy link
Member

lunny commented Apr 12, 2022

The SQL is too long. What's the 500 error log?

@fnetX
Copy link
Contributor Author

fnetX commented Apr 12, 2022

The error log line that appears on the 500 error is the one from the initial comment:

2022/04/10 18:38:42 ...api/v1/repo/issue.go:276:SearchIssues() [E] CountIssues: unable to get one row result when CountIssues, row count=0

@lunny
Copy link
Member

lunny commented Apr 12, 2022

So a Select count(*) SQL with no error but return 0 records. It's wired. https://stackoverflow.com/questions/2552086/does-count-always-return-a-result
I send a PR #19380 to simplify the code to avoid other problem. This PR will guarantee to return 0 even if it's wrong.

@fnetX
Copy link
Contributor Author

fnetX commented Apr 14, 2022

Continuing with debugging, we found out (in chronological order)

  • shorter statements work, longer don't (so indeed, optimizing / shortening it would do improvement short-term)
  • we did a minimal reproduction without Gitea nor XORM, but with PHP which works on one server and not the other (so this issue can be closed if you don't intend to optimize anyway), the issue is with prepared statements (that's why the manually interpolated query did work)
  • and that even with a complete identical MariaDB config and db
  • it seems to be a regression / changed behaviour in MariaDB between (1:10.5.15-0+deb11u1) & (1:10.5.12-0+deb11u1), the update wasn't yet applied in the airgapped staging instance

Edit 2: The error we see from MySQL is Commands out of sync; you can't run this command now

Edit 3:

  • it breaks with 1000 parameters (999 work)

@wxiaoguang
Copy link
Contributor

wxiaoguang commented Apr 14, 2022

Awesome! @lunny that's why I disagree to hide the error.

@fnetX Do you know which version of MariaDB has such bug while which doesn't? Is there any bug report on MariaDB side?

@fnetX
Copy link
Contributor Author

fnetX commented Apr 14, 2022

see Edit above for the error. We tried with 10.5.13, still working. 10.6.7 is broken, too. We'll continue investigating.

@wxiaoguang
Copy link
Contributor

OMG, I have a MariaDB 10.6.7 with 2T data in production 😭 I think I will take a look at this problem too .....

@fnetX
Copy link
Contributor Author

fnetX commented Apr 14, 2022

It's probably a very edge case, though. And it's not breaking any data at least, only the read query. :)

@ashimokawa
Copy link
Contributor

ashimokawa commented Apr 14, 2022

We did more test, even simple queries fail with more that 1000 placeholders since mariadb 10.5.15 or 14 (which we did not test)

@lunny
Copy link
Member

lunny commented Apr 14, 2022

Awesome! @lunny that's why I disagree to hide the error.

@fnetX Do you know which version of MariaDB has such bug while which doesn't? Is there any bug report on MariaDB side?

For a keyword search, repository id placeholder cannot be avoid. Maybe we need to limit it and give a warning in search result page that only display max less 1000 results.

@wxiaoguang
Copy link
Contributor

wxiaoguang commented Apr 14, 2022

Unable to reproduce from my side (PHP PDO has emulated prepared statements ....)

@ashimokawa
Copy link
Contributor

@wxiaoguang
Weird, we can.

It gets even more crazy. Preparing the statement fails with > 65535 parameters with a proper error "too many placeholders", but between 1000-65535 even executing works but fetching results fail.

@wxiaoguang
Copy link
Contributor

wxiaoguang commented Apr 14, 2022

OK, I can reproduce it now with pure SQL. It's really a serious bug .........

I think it's worth to report it to MariaDB issue tracker .....

MariaDB: Server version: 10.6.7-MariaDB-log MariaDB Server

100

PREPARE s FROM 'SELECT COUNT(id) FROM ticket WHERE id IN (?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?)';

EXECUTE s USING 1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,26,27,28,29,30,31,32,33,34,35,36,37,38,39,40,41,42,43,44,45,46,47,48,49,50,51,52,53,54,55,56,57,58,59,60,61,62,63,64,65,66,67,68,69,70,71,72,73,74,75,76,77,78,79,80,81,82,83,84,85,86,87,88,89,90,91,92,93,94,95,96,97,98,99,100;

+-----------+
| COUNT(id) |
+-----------+
|         1 |
+-----------+
1 row in set (0.003 sec)

1001

PREPARE s FROM 'SELECT COUNT(id) FROM ticket WHERE id IN (?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?)';

EXECUTE s USING 1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,26,27,28,29,30,31,32,33,34,35,36,37,38,39,40,41,42,43,44,45,46,47,48,49,50,51,52,53,54,55,56,57,58,59,60,61,62,63,64,65,66,67,68,69,70,71,72,73,74,75,76,77,78,79,80,81,82,83,84,85,86,87,88,89,90,91,92,93,94,95,96,97,98,99,100,101,102,103,104,105,106,107,108,109,110,111,112,113,114,115,116,117,118,119,120,121,122,123,124,125,126,127,128,129,130,131,132,133,134,135,136,137,138,139,140,141,142,143,144,145,146,147,148,149,150,151,152,153,154,155,156,157,158,159,160,161,162,163,164,165,166,167,168,169,170,171,172,173,174,175,176,177,178,179,180,181,182,183,184,185,186,187,188,189,190,191,192,193,194,195,196,197,198,199,200,201,202,203,204,205,206,207,208,209,210,211,212,213,214,215,216,217,218,219,220,221,222,223,224,225,226,227,228,229,230,231,232,233,234,235,236,237,238,239,240,241,242,243,244,245,246,247,248,249,250,251,252,253,254,255,256,257,258,259,260,261,262,263,264,265,266,267,268,269,270,271,272,273,274,275,276,277,278,279,280,281,282,283,284,285,286,287,288,289,290,291,292,293,294,295,296,297,298,299,300,301,302,303,304,305,306,307,308,309,310,311,312,313,314,315,316,317,318,319,320,321,322,323,324,325,326,327,328,329,330,331,332,333,334,335,336,337,338,339,340,341,342,343,344,345,346,347,348,349,350,351,352,353,354,355,356,357,358,359,360,361,362,363,364,365,366,367,368,369,370,371,372,373,374,375,376,377,378,379,380,381,382,383,384,385,386,387,388,389,390,391,392,393,394,395,396,397,398,399,400,401,402,403,404,405,406,407,408,409,410,411,412,413,414,415,416,417,418,419,420,421,422,423,424,425,426,427,428,429,430,431,432,433,434,435,436,437,438,439,440,441,442,443,444,445,446,447,448,449,450,451,452,453,454,455,456,457,458,459,460,461,462,463,464,465,466,467,468,469,470,471,472,473,474,475,476,477,478,479,480,481,482,483,484,485,486,487,488,489,490,491,492,493,494,495,496,497,498,499,500,501,502,503,504,505,506,507,508,509,510,511,512,513,514,515,516,517,518,519,520,521,522,523,524,525,526,527,528,529,530,531,532,533,534,535,536,537,538,539,540,541,542,543,544,545,546,547,548,549,550,551,552,553,554,555,556,557,558,559,560,561,562,563,564,565,566,567,568,569,570,571,572,573,574,575,576,577,578,579,580,581,582,583,584,585,586,587,588,589,590,591,592,593,594,595,596,597,598,599,600,601,602,603,604,605,606,607,608,609,610,611,612,613,614,615,616,617,618,619,620,621,622,623,624,625,626,627,628,629,630,631,632,633,634,635,636,637,638,639,640,641,642,643,644,645,646,647,648,649,650,651,652,653,654,655,656,657,658,659,660,661,662,663,664,665,666,667,668,669,670,671,672,673,674,675,676,677,678,679,680,681,682,683,684,685,686,687,688,689,690,691,692,693,694,695,696,697,698,699,700,701,702,703,704,705,706,707,708,709,710,711,712,713,714,715,716,717,718,719,720,721,722,723,724,725,726,727,728,729,730,731,732,733,734,735,736,737,738,739,740,741,742,743,744,745,746,747,748,749,750,751,752,753,754,755,756,757,758,759,760,761,762,763,764,765,766,767,768,769,770,771,772,773,774,775,776,777,778,779,780,781,782,783,784,785,786,787,788,789,790,791,792,793,794,795,796,797,798,799,800,801,802,803,804,805,806,807,808,809,810,811,812,813,814,815,816,817,818,819,820,821,822,823,824,825,826,827,828,829,830,831,832,833,834,835,836,837,838,839,840,841,842,843,844,845,846,847,848,849,850,851,852,853,854,855,856,857,858,859,860,861,862,863,864,865,866,867,868,869,870,871,872,873,874,875,876,877,878,879,880,881,882,883,884,885,886,887,888,889,890,891,892,893,894,895,896,897,898,899,900,901,902,903,904,905,906,907,908,909,910,911,912,913,914,915,916,917,918,919,920,921,922,923,924,925,926,927,928,929,930,931,932,933,934,935,936,937,938,939,940,941,942,943,944,945,946,947,948,949,950,951,952,953,954,955,956,957,958,959,960,961,962,963,964,965,966,967,968,969,970,971,972,973,974,975,976,977,978,979,980,981,982,983,984,985,986,987,988,989,990,991,992,993,994,995,996,997,998,999,1000,1001;

Query OK, 0 rows affected (0.002 sec)

@ashimokawa
Copy link
Contributor

ashimokawa commented Apr 14, 2022

@wxiaoguang

PHP has emulated prepared statements ....

Not when using mysqli instead of PDO which I did for my tests. They fail as your pure SQL test does.

Also it is strange that it is a recent regression in mariadb since 10.5.13 is still okay and only the recently released 10.5.15 is broken. I do consider this a mariadb bug since there is a proper error when really hitting the placeholder limit (>65535).

@wxiaoguang
Copy link
Contributor

I opened an issue for MairaDB: https://jira.mariadb.org/browse/MDEV-28316

@lunny
Copy link
Member

lunny commented Apr 14, 2022

OK, I can reproduce it now with pure SQL. It's really a serious bug .........

I think it's worth to report it to MariaDB issue tracker .....

MariaDB: Server version: 10.6.7-MariaDB-log MariaDB Server

100

PREPARE s FROM 'SELECT COUNT(id) FROM ticket WHERE id IN (?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?)';

EXECUTE s USING 1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,26,27,28,29,30,31,32,33,34,35,36,37,38,39,40,41,42,43,44,45,46,47,48,49,50,51,52,53,54,55,56,57,58,59,60,61,62,63,64,65,66,67,68,69,70,71,72,73,74,75,76,77,78,79,80,81,82,83,84,85,86,87,88,89,90,91,92,93,94,95,96,97,98,99,100;

+-----------+
| COUNT(id) |
+-----------+
|         1 |
+-----------+
1 row in set (0.003 sec)

1001

PREPARE s FROM 'SELECT COUNT(id) FROM ticket WHERE id IN (?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?)';

EXECUTE s USING 1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,26,27,28,29,30,31,32,33,34,35,36,37,38,39,40,41,42,43,44,45,46,47,48,49,50,51,52,53,54,55,56,57,58,59,60,61,62,63,64,65,66,67,68,69,70,71,72,73,74,75,76,77,78,79,80,81,82,83,84,85,86,87,88,89,90,91,92,93,94,95,96,97,98,99,100,101,102,103,104,105,106,107,108,109,110,111,112,113,114,115,116,117,118,119,120,121,122,123,124,125,126,127,128,129,130,131,132,133,134,135,136,137,138,139,140,141,142,143,144,145,146,147,148,149,150,151,152,153,154,155,156,157,158,159,160,161,162,163,164,165,166,167,168,169,170,171,172,173,174,175,176,177,178,179,180,181,182,183,184,185,186,187,188,189,190,191,192,193,194,195,196,197,198,199,200,201,202,203,204,205,206,207,208,209,210,211,212,213,214,215,216,217,218,219,220,221,222,223,224,225,226,227,228,229,230,231,232,233,234,235,236,237,238,239,240,241,242,243,244,245,246,247,248,249,250,251,252,253,254,255,256,257,258,259,260,261,262,263,264,265,266,267,268,269,270,271,272,273,274,275,276,277,278,279,280,281,282,283,284,285,286,287,288,289,290,291,292,293,294,295,296,297,298,299,300,301,302,303,304,305,306,307,308,309,310,311,312,313,314,315,316,317,318,319,320,321,322,323,324,325,326,327,328,329,330,331,332,333,334,335,336,337,338,339,340,341,342,343,344,345,346,347,348,349,350,351,352,353,354,355,356,357,358,359,360,361,362,363,364,365,366,367,368,369,370,371,372,373,374,375,376,377,378,379,380,381,382,383,384,385,386,387,388,389,390,391,392,393,394,395,396,397,398,399,400,401,402,403,404,405,406,407,408,409,410,411,412,413,414,415,416,417,418,419,420,421,422,423,424,425,426,427,428,429,430,431,432,433,434,435,436,437,438,439,440,441,442,443,444,445,446,447,448,449,450,451,452,453,454,455,456,457,458,459,460,461,462,463,464,465,466,467,468,469,470,471,472,473,474,475,476,477,478,479,480,481,482,483,484,485,486,487,488,489,490,491,492,493,494,495,496,497,498,499,500,501,502,503,504,505,506,507,508,509,510,511,512,513,514,515,516,517,518,519,520,521,522,523,524,525,526,527,528,529,530,531,532,533,534,535,536,537,538,539,540,541,542,543,544,545,546,547,548,549,550,551,552,553,554,555,556,557,558,559,560,561,562,563,564,565,566,567,568,569,570,571,572,573,574,575,576,577,578,579,580,581,582,583,584,585,586,587,588,589,590,591,592,593,594,595,596,597,598,599,600,601,602,603,604,605,606,607,608,609,610,611,612,613,614,615,616,617,618,619,620,621,622,623,624,625,626,627,628,629,630,631,632,633,634,635,636,637,638,639,640,641,642,643,644,645,646,647,648,649,650,651,652,653,654,655,656,657,658,659,660,661,662,663,664,665,666,667,668,669,670,671,672,673,674,675,676,677,678,679,680,681,682,683,684,685,686,687,688,689,690,691,692,693,694,695,696,697,698,699,700,701,702,703,704,705,706,707,708,709,710,711,712,713,714,715,716,717,718,719,720,721,722,723,724,725,726,727,728,729,730,731,732,733,734,735,736,737,738,739,740,741,742,743,744,745,746,747,748,749,750,751,752,753,754,755,756,757,758,759,760,761,762,763,764,765,766,767,768,769,770,771,772,773,774,775,776,777,778,779,780,781,782,783,784,785,786,787,788,789,790,791,792,793,794,795,796,797,798,799,800,801,802,803,804,805,806,807,808,809,810,811,812,813,814,815,816,817,818,819,820,821,822,823,824,825,826,827,828,829,830,831,832,833,834,835,836,837,838,839,840,841,842,843,844,845,846,847,848,849,850,851,852,853,854,855,856,857,858,859,860,861,862,863,864,865,866,867,868,869,870,871,872,873,874,875,876,877,878,879,880,881,882,883,884,885,886,887,888,889,890,891,892,893,894,895,896,897,898,899,900,901,902,903,904,905,906,907,908,909,910,911,912,913,914,915,916,917,918,919,920,921,922,923,924,925,926,927,928,929,930,931,932,933,934,935,936,937,938,939,940,941,942,943,944,945,946,947,948,949,950,951,952,953,954,955,956,957,958,959,960,961,962,963,964,965,966,967,968,969,970,971,972,973,974,975,976,977,978,979,980,981,982,983,984,985,986,987,988,989,990,991,992,993,994,995,996,997,998,999,1000,1001;

Query OK, 0 rows affected (0.002 sec)

Amazing.

@ashimokawa
Copy link
Contributor

Already fixed upstream, but not released yet.

https://jira.mariadb.org/browse/MDEV-27937

@fnetX
Copy link
Contributor Author

fnetX commented Apr 14, 2022

Still, as the next (default) limit will be 65535 parameters, I think this query should be optimized before we'll be hitting that with more growth.

Maybe we need to limit it and give a warning in search result page that only display max less 1000 results.

Doesn't this mean that large instance just won't have working issue dependency linking?

Where do the repo id's come from? All public repos, or all repos where a matching issue was found? Because when searching for some title, it's kinda weird to see soo many results for a simple title query anyway. I'd want to mainly see repos I'm associated with, and only then more results, probably.

I get right that the query does not simply provide all public repos, right?

@Gusted
Copy link
Contributor

Gusted commented Apr 14, 2022

What if we modify the code a bit to avoid passing all the repoIDs:

diff --git a/models/issue.go b/models/issue.go
index b1fa2d02a..1957e570c 100644
--- a/models/issue.go
+++ b/models/issue.go
@@ -1195,6 +1195,7 @@ func GetIssuesByIDs(issueIDs []int64) ([]*Issue, error) {
 type IssuesOptions struct {
        db.ListOptions
        RepoIDs            []int64 // include all repos if empty
+       RepoCond           builder.Cond
        AssigneeID         int64
        PosterID           int64
        MentionedID        int64
@@ -1289,6 +1290,10 @@ func (opts *IssuesOptions) setupSessionNoLimit(sess *xorm.Session) {
                applyReposCondition(sess, opts.RepoIDs)
        }
 
+       if opts.RepoCond != nil {
+               sess.And(opts.RepoCond)
+       }
+
        switch opts.IsClosed {
        case util.OptionalBoolTrue:
                sess.And("issue.is_closed=?", true)
diff --git a/routers/web/repo/issue.go b/routers/web/repo/issue.go
index 3ca193a15..7ab1dae01 100644
--- a/routers/web/repo/issue.go
+++ b/routers/web/repo/issue.go
@@ -2227,7 +2227,7 @@ func SearchIssues(ctx *context.Context) {
                                Page:     ctx.FormInt("page"),
                                PageSize: limit,
                        },
-                       RepoIDs:            repoIDs,
+                       RepoCond:           models.SearchRepositoryCondition(opts),
                        IsClosed:           isClosed,
                        IssueIDs:           issueIDs,
                        IncludedLabelNames: includedLabelNames,

@wxiaoguang
Copy link
Contributor

Doesn't this mean that large instance just won't have working issue dependency linking?

Maybe codeberg is one of the largest one, and usually there shouldn't be so many repo IDs

What if we modify the code a bit (using sub query) to avoid passing all the repoIDs:

It has Pros and Cons.

  • Pros: it seems to resolve the large-amount-repoID problem.
  • Cons: it may have other performance problems and is still difficult to be optimized if there is a performance problem.

I believe knowing the logic fully is the precondition to make optimizations (as the question "where do the repo id's come from"). Maybe it could be optimized from the logic side fundamentally.


And, to handle all similar problems gracefully in future, some improvements:

  1. Make XORM's Count return an error if the result is 0 row.
  2. Make XORM report warnings for SQLs with more than xxxx parameters (eg: 16384), then users/developers can know there is something needed to be refactored, they have enough time to optimize the query before the hard limit comes.

@ashimokawa
Copy link
Contributor

When having more than 65535 parameter there will be a proper error while preparing the statement.

The current behaviour between 1000 and 65535 where 0 rows are returned silently is a bug with unexpected behaviour.
It is different from having just max placeholders down to 999.

@6543 6543 added the performance/speed performance issues with slow downs label Apr 20, 2022
@6543
Copy link
Member

6543 commented Apr 20, 2022

we have to use xorm builder so we create join querys and not In 1....100k querys
where go code insert the constrains

@6543
Copy link
Member

6543 commented Apr 24, 2022

-> #19475

@go-gitea go-gitea locked and limited conversation to collaborators May 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
performance/speed performance issues with slow downs type/bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants