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

[Database Template] Implement Singleton Pattern #209

Merged
merged 10 commits into from
Apr 10, 2024

Conversation

H0llyW00dzZ
Copy link
Contributor

@H0llyW00dzZ H0llyW00dzZ commented Apr 1, 2024

By submitting this pull request, I confirm that my contribution is made under the terms of the MIT license.

Problem/Feature

This pull request implements the Singleton pattern for database connections, which is useful for reusing connections efficiently.

Description of Changes:

  • [+] feat(mysql.tmpl): implement singleton pattern for dbInstance in New function
  • [+] fix(mysql.tmpl): correct dbInstance assignment syntax

Note: This a fix syntax for example it can be used multipe services such as

dbInstance = &service{
		db:       db,
		authuser: NewServiceAuthUser(db), // Initialize the auth user service
	}

Checklist

  • I have self-reviewed the changes being requested
  • I have updated the documentation (if applicable)

@H0llyW00dzZ
Copy link
Contributor Author

tested live in production:

image

json:

{"status":"up","open_connections":"2","idle":"1","max_lifetime_closed":"0","message":"It's healthy","in_use":"1","wait_count":"0","wait_duration":"0s","max_idle_closed":"0"}

Copy link
Collaborator

@briancbarrow briancbarrow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@H0llyW00dzZ this is a good idea, but again we'd need the change applied to all database options.

@H0llyW00dzZ
Copy link
Contributor Author

@briancbarrow how about that MongoDB I've been Implement it ?

@H0llyW00dzZ
Copy link
Contributor Author

btw the ci error lmao

image

@MitchellBerend
Copy link
Contributor

@H0llyW00dzZ I peaked at your action results real quick (for both #209 and #206) and github seems to be having issues right now.

https://www.githubstatus.com/history

@H0llyW00dzZ
Copy link
Contributor Author

@H0llyW00dzZ I peaked at your action results real quick (for both #209 and #206) and github seems to be having issues right now.

https://www.githubstatus.com/history

@MitchellBerend it's unstable, look this a weird failure

image

@H0llyW00dzZ
Copy link
Contributor Author

image

Copy link
Contributor Author

@H0llyW00dzZ H0llyW00dzZ left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

INFO

cmd/template/dbdriver/files/service/mongo.tmpl Outdated Show resolved Hide resolved
@Ujstor
Copy link
Collaborator

Ujstor commented Apr 4, 2024

It seems that GitHub is having issues with actions. I reran manually, and more checks are passing

https://www.githubstatus.com/history

@H0llyW00dzZ
Copy link
Contributor Author

It seems that GitHub is having issues with actions. I reran manually, and more checks are passing

https://www.githubstatus.com/history

this a weird

image

@H0llyW00dzZ
Copy link
Contributor Author

now this one error again lmao

image

@MitchellBerend
Copy link
Contributor

The cache posting issue is resolved in #213 since that removes that action from ci, not sure if this issue is related to yesterday's outage but maybe it's safer to wait for that pr to be merged.

@H0llyW00dzZ
Copy link
Contributor Author

The cache posting issue is resolved in #213 since that removes that action from ci, not sure if this issue is related to yesterday's outage but maybe it's safer to wait for that pr to be merged.

@MitchellBerend The cache posting issue seems to be a race condition because Node.js cannot catch it. Also I don't recommend running CI/CD on Node.js, which is an interpreted language, especially for running lots of CI tasks like this. Unlike compiled languages that can run directly on the machine, such as Go, Node.js adds an extra layer of interpretation.

@Ujstor
Copy link
Collaborator

Ujstor commented Apr 6, 2024

By the end of the weekend, linter fix will be merged

@MitchellBerend
Copy link
Contributor

@MitchellBerend The cache posting issue seems to be a race condition because Node.js cannot catch it. Also I don't recommend running CI/CD on Node.js, which is an interpreted language, especially for running lots of CI tasks like this. Unlike compiled languages that can run directly on the machine, such as Go, Node.js adds an extra layer of interpretation.

Ye I used their github action without actually looking at the internals but I guess when it's being ran this often node is showing it's limitations.

@Ujstor
Copy link
Collaborator

Ujstor commented Apr 6, 2024

Updated workflow will be used on a new commit. I can't run it manually.

@H0llyW00dzZ H0llyW00dzZ mentioned this pull request Apr 7, 2024
@H0llyW00dzZ
Copy link
Contributor Author

H0llyW00dzZ commented Apr 7, 2024

Updated workflow will be used on a new commit. I can't run it manually.

@Ujstor, @briancbarrow this singleton pattern supports Fiber storage as well and provides perfect scalability for rate limiting.

For example:

package database

import (
	"context"
	"database/sql"
	"fmt"
	"log"
	"os"
	"time"

	"github.com/gofiber/fiber/v2"
	"github.com/gofiber/storage/mysql/v2"

	_ "github.com/go-sql-driver/mysql"
	_ "github.com/joho/godotenv/autoload"
)

type Service interface {
	Health() map[string]string
	// RateLimiter returns the [fiber.Storage] interface for rate limiting.
	RateLimiter() fiber.Storage
}

type service struct {
	db          *sql.DB
	ratelimiter fiber.Storage
}

var (
	dbname     = os.Getenv("DB_DATABASE")
	password   = os.Getenv("DB_PASSWORD")
	username   = os.Getenv("DB_USERNAME")
	port       = os.Getenv("DB_PORT")
	host       = os.Getenv("DB_HOST")
	dbInstance *service
)

func New() Service {
	// Reuse Connection
	if dbInstance != nil {
		return dbInstance
	}

	// Opening a driver typically will not attempt to connect to the database.
	db, err := sql.Open("mysql", fmt.Sprintf("%s:%s@tcp(%s:%s)/%s", username, password, host, port, dbname))
	if err != nil {
		// This will not be a connection error, but a DSN parse error or
		// another initialization error.
		log.Fatal(err)
	}
	db.SetConnMaxLifetime(0)
	db.SetMaxIdleConns(50)
	db.SetMaxOpenConns(50)

	dbInstance = &service{
		db: db,
		ratelimiter: mysql.New(mysql.Config{
			Db:         db,
			Reset:      false,
			GCInterval: 10 * time.Second,
			Table:      "rate_limiter",
		}), // Initialize the rate limiter storage
	}
	return dbInstance
}

func (s *service) Health() map[string]string {
	ctx, cancel := context.WithTimeout(context.Background(), 1*time.Second)
	defer cancel()

	err := s.db.PingContext(ctx)
	if err != nil {
		log.Fatalf(fmt.Sprintf("db down: %v", err))
	}

	return map[string]string{
		"message": "It's healthy",
	}
}

// RateLimiter returns the [fiber.Storage] interface for rate limiting.
func (s *service) RateLimiter() fiber.Storage {
	return s.ratelimiter
}

In the database:

Database Table

And it returns nothing when there are no IPs or custom keys, such as API keys, being blocked:

Empty Result

- [+] feat(mysql.tmpl): implement singleton pattern for dbInstance in New function
- [+] fix(mysql.tmpl): correct dbInstance assignment syntax

Note: This a fix syntax for example it can be used multipe services such as
dbInstance = &service{
		db:       db,
		authuser: NewServiceAuthUser(db), // Initialize the auth user service
	}
- [+] feat(mongo.tmpl): implement singleton pattern for MongoDB connection instance
- [+] feat(sqlite.tmpl): implement singleton pattern for database instance creation
- [+] feat(redis.tmpl): implement singleton pattern for database instance creation

Note: This method same as MongoDB must use `sync.Once`
- [+] feat(postgres.tmpl): implement singleton pattern for database instance
@H0llyW00dzZ H0llyW00dzZ changed the title [DATABASE Template] [MYSQL] Implement Singleton Pattern [Database Template] Implement Singleton Pattern Apr 7, 2024
- [+] style(mysql.tmpl): improve readability by formatting dbInstance assignment
- [+] style(sqlite.tmpl): improve readability by formatting dbInstance assignment
@H0llyW00dzZ
Copy link
Contributor Author

H0llyW00dzZ commented Apr 7, 2024

@H0llyW00dzZ this is a good idea, but again we'd need the change applied to all database options.

@briancbarrow all done, also it supported this #209 (comment)

@H0llyW00dzZ
Copy link
Contributor Author

@briancbarrow by the way, for Redis and MongoDB, might not have to set a singleton pattern, I think, because they are both mostly used for caching. Using sync.Once might cause issues because, unlike other databases like MySQL, PostgreSQL, and SQLite, which are stable and can recover from connection issues (I've tested this by killing connections and database servers, and the Go routines can reconnect without using previous connection).

@briancbarrow
Copy link
Collaborator

@briancbarrow by the way, for Redis and MongoDB, might not have to set a singleton pattern, I think, because they are both mostly used for caching. Using sync.Once might cause issues because, unlike other databases like MySQL, PostgreSQL, and SQLite, which are stable and can recover from connection issues (I've tested this by killing connections and database servers, and the Go routines can reconnect without using previous connection).

Okay. Will you remove that code from the Redis and Mongo files?

@H0llyW00dzZ
Copy link
Contributor Author

@briancbarrow done

@H0llyW00dzZ
Copy link
Contributor Author

Also, note that the singleton pattern is more stable and better suited for traditional SQL databases (e.g., MySQL, PostgreSQL, SQLite) compared to using a pooling pattern. On the other hand, the pooling pattern is more stable and better suited for NoSQL databases such as Redis and MongoDB, which are primarily used for caching.

@briancbarrow briancbarrow merged commit f5135e4 into Melkeydev:main Apr 10, 2024
127 checks passed
@H0llyW00dzZ H0llyW00dzZ deleted the mysql-reuse-connection branch April 10, 2024 23:42
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

Successfully merging this pull request may close these issues.

None yet

4 participants