Skip to content

Conversation

@Arkshine
Copy link
Member

Same as alliedmodders/sourcemod#248.

  • This adds new read and write timeout.
  • This unifies all timeout to use the same value.
  • This establishes a default timeout of 60 seconds and considering we don't have proper configuration system like in SM, for existing plugin it may be painful to change timeout, that's why default value is configurable in core.ini.

@Nextra
Copy link
Contributor

Nextra commented Jan 30, 2015

I hope it is okay to put this kind of config into core.ini, it is the best place for it imo. 🚢

@xPaw
Copy link
Contributor

xPaw commented Jan 30, 2015

Why duplicate amx_sql_timeout cvar? Can't amxx plugins read from core.inc as well?

@Nextra
Copy link
Contributor

Nextra commented Jan 30, 2015

@xPaw: This is not a duplication of the amx_sql_timeout variable. This is the default timeout the core MySQL module uses if no timeout is specified (i.e. amx_sql_timeout or any other variable passed into SQL_MakeDbTuple is 0). You can still have your own timeouts when creating a database connection, but the core will mandate a timeout of 60 seconds if no timeout is specified. This is similar to how SourceMod now does things, and I think it's reasonable. Since AMXX has no unified configuration method for databases and many legacy plugins do not specify a timeout in SQL_MakeDbTuple we figured a way to change this mandated timeout is necessary, hence the new entry in core.cfg

@AngeIII: What do you mean? We added the new entry in core.cfg precisely to allow changing the constant.

By the way: It is intended that an undefined timeout and we now have a read/write timeout established.

@pavelthq
Copy link

@Nextra I mean maybe better to write constant "60" into some #define (if once it will be needed to change it will be very easy to change in one place only).

@Arkshine
Copy link
Member Author

@AngeIII Timeout is per database connection. That's something you provide in SQL_MakeDbTuple. If no timeout is provided for a database connection, MySQL module will now set a default timeout of 60 seconds. For convenience, this default value is user-configurable in core.ini.

@Nextra
Copy link
Contributor

Nextra commented Jan 30, 2015

@AngeIII: This is an AMXX module config option which is why it is located in core.ini. I don't quite understand how a compile-time define would be in any way easier than a runtime config option. Nobody wants to recompile a module to change such a simple thing.

@pavelthq
Copy link

@Nextra , @Arkshine thanks, I got it

Nobody wants to recompile a module to change such a simple thing.

Arkshine added a commit that referenced this pull request Jan 30, 2015
…meout

Establish a default timeout for MySQL connectivity
@Arkshine Arkshine merged commit 6319331 into alliedmodders:master Jan 30, 2015
@Arkshine Arkshine deleted the feature/mysql-rw-and-default-timeout branch January 30, 2015 14:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants