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

NOISSUE - Add Things Migration #2

Open
wants to merge 41 commits into
base: main
Choose a base branch
from

Conversation

rodneyosodo
Copy link
Member

@rodneyosodo rodneyosodo commented Mar 17, 2023

Signed-off-by: rodneyosodo [email protected]

KNOWN ISSUES

  • We have an issue when importing version 0.14.0 - Standby mode was to create a branch on http://github.com/rodneyosodo/mainflux/pkg/sdk/go v0.0.0-20230321144058-d764206b0afa
  • When exporting connections of things and channels from 0.13.0 it takes a little bit longer time depending on the number of things and channels connected
  • When inserting the connections from 0.13.0 to 0.14.0 it takes some time depending on the number of connections. If is large it will timeout the token i.e takes more than 15mins provided

@rodneyosodo rodneyosodo marked this pull request as ready for review March 21, 2023 16:22
@@ -0,0 +1,39 @@
package main
Copy link
Member

Choose a reason for hiding this comment

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

No need for a migrate subdirectory, cmd/main.go is just fine.

func connectToThingsDB(dbConfig mf13thingspostgres.Config) *sqlx.DB {
db, err := mf13thingspostgres.Connect(dbConfig)
if err != nil {
log.Panic(fmt.Errorf("Failed to connect to things postgres: %s", err))
Copy link
Member

Choose a reason for hiding this comment

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

Use log.Fatalf instead.

// Root Flags
rootCmd.PersistentFlags().StringVarP(&cfg.FromVersion, "fromversion", "f", "0.13.0", "mainflux version you want to migrate from")
rootCmd.PersistentFlags().StringVarP(&cfg.ToVersion, "toversion", "t", "0.14.0", "mainflux version you want to migrate to")
rootCmd.PersistentFlags().StringVarP(&cfg.Operation, "operation", "o", "export", "export dataor import data to a new mainflux deployment")
Copy link
Member

Choose a reason for hiding this comment

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

Fix typo.

}
o := limit
limit = util.UpdateLimit(usersPage.Total)
for o < usersPage.Total {
Copy link
Member

Choose a reason for hiding this comment

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

The idea of fetching everything from the DB to the RAM. That's the point of pagination. This should be more like a pipe where we stream (channel) the batch, and publish it to the next phase to process. Bear in mind that we'll need to remember "last saved" so in case something goes wrong we don't need to start over.

@rodneyosodo rodneyosodo changed the title NOISSUE - Add Things Migration From 0.13.0 To 0.14.0 NOISSUE - Add Things Migration Apr 26, 2023
Signed-off-by: rodneyosodo <[email protected]>
Signed-off-by: rodneyosodo <[email protected]>
Signed-off-by: rodneyosodo <[email protected]>
Signed-off-by: rodneyosodo <[email protected]>
Signed-off-by: rodneyosodo <[email protected]>
Signed-off-by: rodneyosodo <[email protected]>
Signed-off-by: rodneyosodo <[email protected]>
Signed-off-by: rodneyosodo <[email protected]>
Signed-off-by: rodneyosodo <[email protected]>
Signed-off-by: rodneyosodo <[email protected]>
Signed-off-by: rodneyosodo <[email protected]>
Signed-off-by: rodneyosodo <[email protected]>
Signed-off-by: rodneyosodo <[email protected]>
Signed-off-by: rodneyosodo <[email protected]>
Signed-off-by: rodneyosodo <[email protected]>
Signed-off-by: rodneyosodo <[email protected]>
Signed-off-by: rodneyosodo <[email protected]>
@rodneyosodo rodneyosodo force-pushed the NOISSUE-thingsmigration branch from 1c3819f to 809adf8 Compare April 28, 2023 09:50
Signed-off-by: rodneyosodo <[email protected]>
cmd/main.go Outdated
)

const (
migrationDuration = 1 * time.Hour
Copy link
Contributor

Choose a reason for hiding this comment

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

this might not be a good idea to set a fixed value because we don't know how long it could take for different sizes of migrations

internal/file.go Outdated Show resolved Hide resolved
internal/metadata.go Outdated Show resolved Hide resolved
internal/metadata.go Outdated Show resolved Hide resolved
Comment on lines 14 to 21
switch {
case total <= limit1000:
return defLimit
case total <= limit10000:
return limit1000
case total <= limit100000:
return limit10000
case total <= limit1000000:
Copy link
Contributor

Choose a reason for hiding this comment

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

this can be reduced a mathematical statement

migrate/migrate.go Outdated Show resolved Hide resolved

func Migrate(ctx context.Context, cfg migrations.Config, logger logger.Logger) {
go func() {
log.Println(http.ListenAndServe("localhost:6060", nil))
Copy link
Contributor

Choose a reason for hiding this comment

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

use mainflux logger

migrate/migrate.go Outdated Show resolved Hide resolved
migrate/migrate.go Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@SammyOina
Copy link
Contributor

regarding the token timeout, you can add to documentation on how to change env var to create a longer duration token for migration purposes

Signed-off-by: rodneyosodo <[email protected]>
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.

3 participants