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

MySQL / MOO SQL bf? #35

Open
rwsmith opened this issue Dec 31, 2020 · 8 comments
Open

MySQL / MOO SQL bf? #35

rwsmith opened this issue Dec 31, 2020 · 8 comments

Comments

@rwsmith
Copy link

rwsmith commented Dec 31, 2020

Hi,

Wanted to thank lisdude, Todd, and all contributors for the amazing work done on Stunt/ToastStunt. I notice SQLite is chosen rather than MySQL, even though MOOSQL has enjoyed some popularity over the years. MOOSQL makes blocking calls, which is a bummer, and also doesn't support prepared statements.

Two things:

  • Just curious why SQLite was chosen over, say, MySQL or Postgres? Not trying to grind anyone's gears nor start a flame war on RDBMS, just wondering, if there was any reasons here.
  • Let's say I came up with a stable, non-blocking patch (using background.cc) that is MOOSQL bf compatible (and also supported prepared statements) for MySQL. Is such a patch welcome in ToastStunt or is this against the philosophy of ToastStunt in some fashion?

Thanks again, for the amazing work.

Best,

Ryan

@toddsundsted
Copy link

i'm probably not the best person to ask about the philosophy of toaststunt, but i would have integrated sqlite into stunt, instead of postgresql or mysql because i wanted fewer moving parts and fewer external dependencies. one of the things i loved about the original lambdamoo was that i could download the code, run configure, run make, and then run the server -- no devops required. it made it easy to get it up and running, and i'm convinced that low barrier added to the popularity. sqlite would have been an external build dependency (although it could have been integrated into the project like i did with other packages) but it isn't an external service that has to be managed.

there's nothing like consistency... i realized as i responded here that this is exactly why i picked sqlite for https://github.com/toddsundsted/ktistec

@distantorigin
Copy link
Collaborator

distantorigin commented Dec 31, 2020

Let's say I came up with a stable, non-blocking patch (using background.cc) that is MOOSQL bf compatible (and also supported prepared statements) for MySQL. Is such a patch welcome in ToastStunt or is this against the philosophy of ToastStunt in some fashion?

This is not against the philosophy of ToastStunt and would be welcome as a contribution. SQLite was originally written as an extension for LambdaMOO-Stunt and it was a no-brainer bringing it into the fold since it's been invaluable on Miriani and other MOOs for the aforementioned reasons from Todd. We originally started on MySQL but transitioned some years ago.

My chief concern would be brushing against the builtin function limit; I'm unsure where we stand on that and #27 still remains open, pending discussion.

@tvdijen
Copy link

tvdijen commented Dec 31, 2020

i'm probably not the best person to ask about the philosophy of toaststunt, but i would have integrated sqlite into stunt, instead of postgresql or mysql because i wanted fewer moving parts and fewer external dependencies. one of the things i loved about the original lambdamoo was that i could download the code, run configure, run make, and then run the server -- no devops required. it made it easy to get it up and running, and i'm convinced that low barrier added to the popularity. sqlite would have been an external build dependency (although it could have been integrated into the project like i did with other packages) but it isn't an external service that has to be managed.

I think the gdbdb-extension deserved more attention in this regard as well... It's kinda prehistoric nowadays, but it still serves it's purpose very well!

@lisdude
Copy link
Owner

lisdude commented Jan 1, 2021

Yep, everybody above pretty much echo my thoughts. I wanted to avoid having to configure, secure, and manage another service with data in another location. I like the containment that SQLite offers with the databases being accessible from the same location that the MOO already has access to with FIO. Plus the sheer simplicity of it felt more... MOOey.

Did you intend to have both at the same time? It seems a bit redundant, like using both FUP and FIO. I'd rather see it as something in options.h or a CMake option so you could choose between one or the other and avoid having another large dependency. We do seem to be accumulating them!

Also watch out. I seem to remember the MOOSQL patch that was floating around had a fairly serious issue. It was either a large memory leak or a crash... If you're interested, I can post the one I used with Miriani all those years ago. It should have whatever it was fixed. Remembering things sure is hard...

@rwsmith
Copy link
Author

rwsmith commented Jan 1, 2021

Thanks folks for all the input! And greatly appreciate the warning on MOOSQL -- I remember getting bit by a memory leak on an older version of the patch.

Did you intend to have both at the same time?

My intention would be either or. I definitely understand the dependency creep and wanting to keep them to a minimum. I've been looking into SQLite and I think migrating to it is likely what I will do, but that won't stop a hacker from tinkering :)! FWIW, there is a blocking MOOSQL patch for Stunt here.


I looked more into the MySQL C APIs, and because we are now introducing multiple threads & how MySQL C APIs work, I think we would be left with at least these options:

  • create one connection per query (what my current tinkering does)
  • use a mutex lock per MySQL connection, like the docs say
  • checkout / check-in connections to and from a connection pool (what I've seen done in big important services).

The SQLite APIs did look a little bit nicer overall.

I came across a very interesting library named LibZDB. It looks like a thread-safe, relatively simple API that supports connection pooling/querying for SQLite, Postgres and MySQL.

I am wondering if anyone has used it / has thoughts on LibZDB?

@biscuitWizard
Copy link
Contributor

biscuitWizard commented Nov 17, 2021

I've addressed this and added a PR for adding postgres support.

Edit:
Notably my version only supports postgres, is non-blocking, and uses parameterization. It also only supports connecting to a single database, which may be less control than what other users may want from a SQL implementation in MOO, but it's the smallest possible package for the production requirements we have.

It also made the implementation simpler, which means less bugs.

@biscuitWizard
Copy link
Contributor

biscuitWizard commented Dec 20, 2021

After looking into LibZDB, I'm here to report my findings:

LibZDB is not good software, it doesn't handle exceptions properly and frequently results in an uncatchable segfault.

You can replicate this with providing it any ill-formatted SQL URL. catch(...) does not work, nor do the provided TRY/CATCH macros provided with the library. It will always result in a panic crash.

Unfortunately, I cannot recommend this library at all, it wasn't written to any acceptable production standard.

Here's the code I used to validate my findings:

url = URL_new(arglist.v.list[1].v.str);
        if (url)
        {
            try {
                TRY
                {
                    pool = ConnectionPool_new(url);    
                    if (!pool) {
                        ret->type = TYPE_ERR;
                        ret->v.err = E_PERM;    
                    }
                }
                ELSE {
                    ret->type = TYPE_ERR;
                    ret->v.err = E_PERM;
                }
                END_TRY;
            } catch (...) 
            {
                ret->type = TYPE_ERR;
                ret->v.err = E_PERM;
            }
        } else {
            ret->type = TYPE_ERR;
            ret->v.err = E_INVARG;
        }

if arg[1] is "butts", it catches that and handles gracefully. But a 'mock good' url fails and causes a full sig11.

I am fully against integrating anything that cannot handle bad cases or cannot be hardened. It should be very, very hard to cause a MOO to crash, and this hasn't even gotten to stages where I want to test what happens if SQL connections are disrupted or loss.

@biscuitWizard
Copy link
Contributor

I have another update, and it's good news this time! After talking with Tildeslash, we identified that the problem is opening connections has to be done on the main thread. After validating that and correcting my botched understanding of the written documentation, I actually have ZDB working and hardened.

The First Room
This is all there is right now.
ANSI Version 2.5 is currently active.  Type "?ansi-intro" for more information.
Your previous connection was before we started keeping track.
There is new news.  Type `news' to read all news or `news new' to read just new news.
;sql_open("postgresql://localhost:5432/moo?user=postgres&password=test")
=> 1
;sql_open("postgresql://localhost:5432/moo?user=postgres&password=test")
=> 1
;sql_open("meow meow meow")
=> "Invakid URL"
;sql_open("whatever")
=> "Invakid URL"
;sql_connections()
=> [1 -> "postgresql://localhost:5432/moo?user=postgres&password=test"]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants