| Time |
Nick |
Message |
| 00:05 |
|
Baytuch joined #minetest-dev |
| 00:22 |
|
proller joined #minetest-dev |
| 01:44 |
|
LandarVargan joined #minetest-dev |
| 03:43 |
|
dzho joined #minetest-dev |
| 03:43 |
|
dzho joined #minetest-dev |
| 05:00 |
|
MTDiscord joined #minetest-dev |
| 05:36 |
|
calcul0n joined #minetest-dev |
| 09:24 |
|
appguru joined #minetest-dev |
| 12:05 |
|
Baytuch joined #minetest-dev |
| 12:40 |
|
proller joined #minetest-dev |
| 12:51 |
|
BuckarooBanzai0 joined #minetest-dev |
| 12:52 |
|
jojje joined #minetest-dev |
| 12:56 |
|
Noisytoot joined #minetest-dev |
| 13:00 |
|
Pexin joined #minetest-dev |
| 13:00 |
|
Thomas-S joined #minetest-dev |
| 13:00 |
|
basxto joined #minetest-dev |
| 13:00 |
|
nore joined #minetest-dev |
| 13:09 |
|
Baytuch_2 joined #minetest-dev |
| 13:19 |
|
Baytuch joined #minetest-dev |
| 13:32 |
|
Baytuch joined #minetest-dev |
| 14:37 |
|
Baytuch joined #minetest-dev |
| 15:51 |
|
appguru joined #minetest-dev |
| 18:12 |
|
Desour joined #minetest-dev |
| 18:32 |
|
appguru joined #minetest-dev |
| 19:08 |
|
book` joined #minetest-dev |
| 19:40 |
|
calcul0n joined #minetest-dev |
| 19:47 |
cfnblx |
For those of you who don't Discord -- FYI, I submitted a PR for the MariaDB backend: https://github.com/minetest/minetest/pull/13266 |
| 19:48 |
cfnblx |
wsor / sfan5 have tagged it, and it has passed all checks. I pulled the current master just prior to pushing my commits, so it should be able to be merged in very simply |
| 19:49 |
cfnblx |
I've had a couple friends look over to code to make sure it didn't introduce any errors, and everyone has given it a thumbs up. Hoping you guys will do the same. |
| 19:54 |
Krock |
Desour: about #13264 - we're already drawing the tooltips ourselves (GUIFormSpecMenu::showTooltip). What's the difference in the PR? |
| 19:54 |
pgimeno |
https://github.com/minetest/minetest/pull/13264 -- Draw irrlicht tooltips ourself by Desour |
| 19:56 |
Krock |
cfnblx: what exactly is the motivation behind this change? better integration with existing database servers? |
| 19:56 |
Desour |
Krock: irrlicht iguielements also have a tooltip (the one set by setToolTip). our table[] elements use this. these tooltips are usually drawn in the guienvironment |
| 19:56 |
Desour |
see also the github code comment I made |
| 19:57 |
Krock |
I saw the comment but did not see any such Irrlicht tooltips |
| 19:58 |
ROllerozxa |
hover over any of the icons in the multiplayer server list? |
| 19:58 |
Desour |
grep for setToolTip, it's done in gui/guiTable.cpp |
| 19:58 |
Krock |
yes but I mean in the code. where exactly is it rendered |
| 19:58 |
Desour |
ah, see the linked irrlicht PR |
| 19:59 |
Krock |
oh. that's the part I missed. thanks. |
| 20:03 |
cfnblx |
Krock: yes - to add MariaDB as an option for database backends. It fully supports map, auth, player, and mod storage in a single database and is fully tested. |
| 20:04 |
Krock |
Desour: you could as well skip the tooltip drawing if `getSkin()->getColor(EGDC_TOOLTIP) == 0` in which case there would not be a text in first place.. but if we're not using the Irrlicht feature we could as well just purge everything as suggested |
| 20:06 |
cfnblx |
Similar to PostgreSQL, I can't imagine many people would want to go through the trouble of setting it up for a client-only installation, so it's mostly intended for people running servers, especially those who may already have large investments into learning/running MySQL/MariaDB like myself. |
| 20:06 |
Krock |
yes that makes entirely sense. thanks. |
| 20:06 |
cfnblx |
Obviously I can continue to reintegrate my code and build from source, but it would be wonderful if next release could include my PR so I could actually use it on my public servers. |
| 20:07 |
cfnblx |
(without all that extra trouble, I mean) |
| 20:07 |
Krock |
problem is that a new release should be out for months already but it's still the same things blocking it |
| 20:08 |
cfnblx |
Well, this one is hopefully a no-brainer as I've done all the work to make sure it's painless as possible for you guys. |
| 20:09 |
Krock |
cfnblx: src/database/database-mariadb.h and .cpp lack an #if USE_MARIADB guard. compiling will fail if these headers are not found |
| 20:09 |
Krock |
although I find it interesting the the build bot pased |
| 20:10 |
Krock |
actually nvm it did not start in first place due to first-time contributor status |
| 20:10 |
Krock |
(nvm x2 because the build bot status is not shown in the "commits" tab but it did eventually run in the past) |
| 20:11 |
Krock |
either way I'll add it to the meeting points to decide its roadmap |
| 20:11 |
cfnblx |
Uhm, no they're not |
| 20:11 |
nrz_ |
Interesting, i should check query quality and schème |
| 20:13 |
Desour |
Krock: FYI, I tried setting the font to NULL, but it falls back to the default font. I haven't tried setting the color to 0, as I wasn't sure if then the tooltips might be just uglily invisible. anyway, such solutions also seemed a bit hacky to me |
| 20:13 |
cfnblx |
Wait, I stand corrected. .cpp does have the condition, .h does not |
| 20:14 |
cfnblx |
database-postgre.h does not have the condition. Are you sure mine should? |
| 20:14 |
Krock |
Desour: setting the color to 0 will make the tooltips render anyway. it was a suggestion to change the behavior in IrrlichtMt |
| 20:14 |
ROllerozxa |
well technically the header won't be included if USE_MARIADB is false since it's walled off in server.cpp |
| 20:14 |
Desour |
ah, I see |
| 20:14 |
cfnblx |
right, so why do I need to add it to the .h? |
| 20:15 |
Krock |
cfnblx: I did not check the include paths. if the header is protected from inclusion properly (where it is actually needed) it serves its purpose too |
| 20:15 |
cfnblx |
yes, it is |
| 20:15 |
ROllerozxa |
ok so redis' and leveldb's header files are gated but not postgre |
| 20:15 |
cfnblx |
I spent a month on this project. It was not rushed, nor pushed prematuretly. |
| 20:15 |
Krock |
src/map.cpp - it's included and protected there. good to know, thanks! |
| 20:16 |
ROllerozxa |
that maybe should be sorted out |
| 20:18 |
Krock |
yes there's some inconsistencies which might have contributed to my hasty remark about the guards. even still, there's way more inconsistent code in Minetes |
| 20:18 |
MTDiscord |
<luatic> The code is repeating the pattern try { ... } catch (sql::SQLException &e) { std::string msg = std::string("[MariaDB] Error: ") + e.what(); std::cout << msg << std::endl; throw DatabaseException(msg); } a lot. I'd consider looking into how to deduplicate this, using a macro as a last resort of all else fails. |
| 20:18 |
MTDiscord |
<luatic> s/of/if |
| 20:19 |
cfnblx |
https://github.com/minetest/minetest/pull/13266/commits/54ad5654db1ba8fd191c12b72fa3725a073ac830 |
| 20:19 |
cfnblx |
resolved inconsistency |
| 20:20 |
MTDiscord |
<luatic> btw, does MariaDB not have a CREATE TABLE IF NOT EXISTS? because I see plenty of if (!tableExists(...)) createTable(...) in the code. |
| 20:21 |
MTDiscord |
<caffeinatedblocks> It does, I just chose to make it a separate query. |
| 20:21 |
MTDiscord |
<ROllerozxa> CREATE TABLE IF NOT EXISTS does in fact exist in mariadb, I believe |
| 20:21 |
Krock |
the PR will be discussed this Sunday if the meeting takes place, and reviewers should appear automatically |
| 20:21 |
MTDiscord |
<luatic> why? |
| 20:22 |
MTDiscord |
<caffeinatedblocks> Why not? |
| 20:22 |
MTDiscord |
<caffeinatedblocks> Technically, my way is less expensive. |
| 20:22 |
MTDiscord |
<caffeinatedblocks> (bandwidth/network wise) |
| 20:22 |
Krock |
does it matter for one-time queries? |
| 20:22 |
MTDiscord |
<paradust> not at all |
| 20:23 |
MTDiscord |
<caffeinatedblocks> it only sends a full table definition over the wire if the table actually doesn't exist |
| 20:23 |
MTDiscord |
<paradust> every query needs to grab and release a table lock |
| 20:23 |
MTDiscord |
<ROllerozxa> more queries is less expensive? |
| 20:23 |
MTDiscord |
<paradust> that's going to be 100x more expensive than a few extra bytes on the wire |
| 20:23 |
|
Baytuch joined #minetest-dev |
| 20:23 |
MTDiscord |
<ROllerozxa> yeah |
| 20:23 |
MTDiscord |
<caffeinatedblocks> It's a one time thing at startup. |
| 20:23 |
MTDiscord |
<luatic> yeah, and that's why I'd go for the shorter version that does less queries (if the tables don't exist) |
| 20:24 |
MTDiscord |
<caffeinatedblocks> But then you're sending longer queries for the life of the database |
| 20:24 |
MTDiscord |
<caffeinatedblocks> Mine sends one extra query the very first time you ever start a new world/database. |
| 20:24 |
MTDiscord |
<luatic> sounds like premature optimization |
| 20:24 |
MTDiscord |
<caffeinatedblocks> Your way would send a full table creation query every single startup for the life of the database. |
| 20:24 |
MTDiscord |
<paradust> yea if it's only done once, this doesn't matter much |
| 20:24 |
MTDiscord |
<luatic> yes, and that's completely negligible |
| 20:25 |
MTDiscord |
<caffeinatedblocks> Mine sends two once, then only a short "does this exist?" forever |
| 20:25 |
MTDiscord |
<luatic> which is why the shorter way should be preferred, IMO |
| 20:25 |
MTDiscord |
<caffeinatedblocks> @luatic how many production MariaDB servers are you running with hundreds of tables and millions of rows of data? |
| 20:26 |
MTDiscord |
<luatic> irrelevant authority argument immediately dismissed |
| 20:26 |
MTDiscord |
<caffeinatedblocks> My ways are tried and true |
| 20:26 |
MTDiscord |
<luatic> ... |
| 20:26 |
MTDiscord |
<ROllerozxa> I would hope it aborts as soon as it realises the table already exists, you'll save code lines, indentation levels and query count |
| 20:26 |
MTDiscord |
<caffeinatedblocks> You're welcome to argue on the merit of "what you think" if you prefer |
| 20:26 |
Krock |
does network traffic of 500 characters matter when you're constantly retrieving mapblocks in the 16k+ magnitude from the database afterwards? |
| 20:26 |
MTDiscord |
<luatic> it doesn't |
| 20:26 |
MTDiscord |
<luatic> this is premature optimization at the cost of code quality |
| 20:27 |
MTDiscord |
<luatic> if you really want to shorten your queries, why don't you remove those NOT NULL constraints from x, y, z - they are redundant with the primary key constraint? :trollface: |
| 20:29 |
MTDiscord |
<ROllerozxa> or utf8mb4_unicode_ci everywhere |
| 20:30 |
MTDiscord |
<luatic> @caffeinatedblocks btw, all your error handling is using std::cout << msg << std::endl; - I think you should prefer using one of Minetest's logging streams, probably errorstream here. |
| 20:34 |
MTDiscord |
<caffeinatedblocks> Actually, I used that inconsistently it seems. |
| 20:34 |
MTDiscord |
<caffeinatedblocks> I intended to replace all of them with errorstream like I did in the Database_MariaDB constructor, but apparently I forgot that |
| 20:36 |
Krock |
also you wouldn't need to first write the error message into `std::string` before feeding it into `errorstream` |
| 20:37 |
Krock |
unless there's a log printing race condition that I am not aware of.. |
| 20:37 |
MTDiscord |
<caffeinatedblocks> allows concatenation with e.what() |
| 20:37 |
MTDiscord |
<caffeinatedblocks> (for thrown exception message) |
| 20:37 |
Krock |
errorstream << "test test " << e.what() << std::endl; works just fine |
| 20:37 |
MTDiscord |
<caffeinatedblocks> I suppose I could just output to errorstream and throw a generic exception ("MariaDB fail") |
| 20:38 |
MTDiscord |
<caffeinatedblocks> I'm currently outputting full error message with e.what() to both cout/errorstream and also into the exception itself (which may or may not be useful) |
| 20:40 |
Krock |
I see. the exceptions need to be re-thrown as a Minetest type.. |
| 20:44 |
Krock |
In case you'd like to change it quickly in the future and not repeat too much code, you could resort to a copy&paste tool like Macros: https://pastebin.com/raw/nDhNsbHd (just mentioning in case you'd be interested) |
| 20:45 |
|
Baytuch joined #minetest-dev |
| 20:47 |
MTDiscord |
<caffeinatedblocks> I've opted to simply do what I said above |
| 20:54 |
MTDiscord |
<caffeinatedblocks> In following the same rules I've learned in marriage ("do you want to be RIGHT or HAPPY?"), here you go: |
| 20:54 |
MTDiscord |
<caffeinatedblocks> https://github.com/minetest/minetest/pull/13266/commits/c02a8377a9b630d71cac26e3ba9d71c64fc30dc0 |
| 20:55 |
MTDiscord |
<caffeinatedblocks> Have I now appeased the powers that be? |
| 20:58 |
|
ShadowBot joined #minetest-dev |
| 20:59 |
Krock |
I should resort to github reviews again. They should leave me less headroom for premature/rushed comments. thanks for the errorstream change. The PR will be discussed at least this week again, possibly including other opinions |
| 21:00 |
Krock |
it's easier to work with when there's an entire review than just random comments |
| 21:02 |
MTDiscord |
<caffeinatedblocks> I've removed the unnecessary separate existence check queries for database and tables, redirected all error output to errorstream, modified all exceptions to simply throw "MariaDB error" (since error is output to errorstream already anyway), and also removed the NOT NULL definitions as per luatic's comments |
| 21:03 |
|
Baytuch joined #minetest-dev |
| 21:05 |
Krock |
I think that particular comment from luatic was not meant serious (considering the "trollface" reaction) |
| 21:08 |
MTDiscord |
<caffeinatedblocks> When your boss is fresh out of college, and all you have is more time and experience in the field than he's been alive, what else do you do? Argue with him? Nah, just shut up and do as asked. He'll figure it out eventually. |
| 21:10 |
Krock |
I'd call it unprofessional like my initial blunder on the header guard |
| 21:13 |
MTDiscord |
<caffeinatedblocks> Personally, I'd have kept the NOT NULL. In my experience, indexes can change, and forgetting the implicit NOT NULL can result in catastrophe if/when that happens. |
| 21:15 |
MTDiscord |
<caffeinatedblocks> Well, personally, errorstream redirection aside, I'd have kept everything as-is. "Code quality" is not compromised by a if (!tableExists) -- if anything, it tells me exactly what is happening without having to scroll up to be sure "IF NOT EXISTS" was actually included in the statement. |
| 21:16 |
proller |
somebody make benchmarks of all dbbackends ? |
| 21:18 |
MTDiscord |
<caffeinatedblocks> I benchmarked mine using an emerge script |
| 21:19 |
MTDiscord |
<caffeinatedblocks> SQLite/Postgre operates about 53% faster with an emerge from (-64, -64, -64) to (64, 64, 64), but all other benchmarks are/were purely observational/anecdotal |
| 21:20 |
MTDiscord |
<caffeinatedblocks> With regard to that additional latency, I spoke with the CTO of MariaDB and he opened this issue to have that addressed in a later release: https://jira.mariadb.org/browse/MDEV-30736 |
| 21:20 |
proller |
whats about redis or leveldb ? |
| 21:20 |
MTDiscord |
<caffeinatedblocks> I did not test either |
| 21:21 |
MTDiscord |
<caffeinatedblocks> I just compared to the default (sqlite) and the "competition" (postgre) |
| 21:21 |
proller |
or "slow" and "very slow" ? |
| 21:21 |
proller |
why do not try faster variants? |
| 21:22 |
MTDiscord |
<caffeinatedblocks> SQLite and Postgre are actually both on par with each other as far as the emerge goes. SQLite finished 2 seconds faster. |
| 21:23 |
MTDiscord |
<caffeinatedblocks> MariaDB, on the other hand, has the ability to run fully in memory like REDIS, unlike SQLite/Postgre (without using, for example, memfs/ramdisk) |
| 21:23 |
MTDiscord |
<paradust> does minetest really store blockdata in sql? good lord |
| 21:24 |
MTDiscord |
<paradust> i thought it was just for user/auth records, but i've never looked into it |
| 21:24 |
proller |
8) |
| 21:24 |
MTDiscord |
<caffeinatedblocks> (though I currently have it hard-coded for InnoDB, that could be easily changed to any of the other many supported storage engines, or you could even write your own storage engine) |
| 21:26 |
MTDiscord |
<caffeinatedblocks> With additional optimization tweaks (which make InnoDB less ACID compliant), you can also increase current performance by setting innodb_flush_log_at_trx_commit = 2 |
| 21:26 |
proller |
somebody should make some standard bench code for all available backends, for write-read thousands blocks and print times/speeds |
| 21:27 |
MTDiscord |
<caffeinatedblocks> That's difficult. |
| 21:27 |
proller |
blockdata should not be stored in any sql and cloud bases |
| 21:27 |
MTDiscord |
<caffeinatedblocks> When working with the fellas over at MariaDB to develop this backend, we came up with some really creative tools to benchmark in a hypothetical sense... Minetest doesn't come anywhere optimal performance. |
| 21:30 |
proller |
now leveldb should be fastest |
| 21:31 |
proller |
and adding rocksdb may make faster |
| 21:31 |
|
Baytuch joined #minetest-dev |
| 21:32 |
MTDiscord |
<ROllerozxa> doesn't leveldb have... pretty serious reliability issues |
| 21:32 |
MTDiscord |
<ROllerozxa> I'd wanna know if rocksdb could solve them though, leveldb is in fact pretty neat |
| 21:32 |
proller |
also it almost frozen development |
| 21:33 |
proller |
ROllerozxa> thats why and i have hand crafted repair tool |
| 21:34 |
proller |
it works fine if dont power off server every day |
| 21:35 |
MTDiscord |
<caffeinatedblocks> https://dpaste.com/4XU5EJQ98 |
| 21:35 |
MTDiscord |
<caffeinatedblocks> Here's a script written by a fellow over at MariaDB |
| 21:36 |
MTDiscord |
<caffeinatedblocks> On my server, that takes 218643.792236 ms to complete, whereas emerge blocks (-64, -64, -64) (64, 64, 64) takes 282s |
| 21:36 |
MTDiscord |
<caffeinatedblocks> the extra 64 seconds are minetest overhead/latency |
| 21:38 |
MTDiscord |
<caffeinatedblocks> Then again, I seriously doubt anyone is running a server anywhere near mine in terms of hardware performance |
| 21:39 |
MTDiscord |
<caffeinatedblocks> My server's a bad mammajamma and has some seriously fast NVMes: https://dpaste.com/B4YFMCWTU |
| 21:42 |
MTDiscord |
<caffeinatedblocks> (and that's not even bare-metal performance... that includes over head from VMware's Paravirtual SCSI controller) |
| 21:43 |
MTDiscord |
<caffeinatedblocks> That said, if anyone comes up with some tools to benchmark all available backends, I'd be happy to try them out 🙂 |
| 21:56 |
pgimeno |
is there any DB backend that can keep each world database in the world directory, besides sqlite? |
| 21:58 |
MTDiscord |
<Jonathon> leveldb |
| 21:58 |
MTDiscord |
<Jonathon> but i wouldnt use it unless you love data loss/restoring from backups |
| 21:59 |
pgimeno |
I'd consider firebird as a backend then |
| 22:56 |
|
Baytuch joined #minetest-dev |
| 23:08 |
|
proller joined #minetest-dev |
| 23:46 |
|
kilbith joined #minetest-dev |
| 23:50 |
|
Baytuch joined #minetest-dev |
| 23:53 |
|
Baytuch joined #minetest-dev |
