Skip to content

Conversation

@Praveenrajmani
Copy link
Contributor

@Praveenrajmani Praveenrajmani commented May 11, 2023

Description

Currently, we will miss the events if MinIO server receives the API requests before the notification sub-system is initialized and the bucket target rules are populated.

This PR will freeze the APIs until these subsystems get completely initialized.

Motivation and Context

  • No events should me missed on server restarts
  • No interim events should be missed between the bootstrap and notification-subsystem initialization.

How to test this PR?

  • Configure bucket notifications with or without queue_dir
  • In master apply the following diff to simulate the config subsystem and bucket target rules time lag.
diff --git a/cmd/server-main.go b/cmd/server-main.go
index e212e13e2..4f5e047e8 100644
--- a/cmd/server-main.go
+++ b/cmd/server-main.go
@@ -417,6 +417,8 @@ func initServer(ctx context.Context, newObject ObjectLayer) error {
 
        r := rand.New(rand.NewSource(time.Now().UnixNano()))
 
+       time.Sleep(3*time.Minute)
+
        for {
                select {
                case <-ctx.Done():
  • Start MinIO - it will be sleeping for 3 minutes.
  • Copy few objects to MinIO - it will be successful but no events can be seen or persisted for those objects.
  • With this change in the PR, the API's will freezed until the targets and configs are initialized and then unfreezes once done. Here, we won't miss any events.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Optimization (provides speedup with no functional changes)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • Fixes a regression (If yes, please add commit-id or PR # here)
  • Unit tests added/updated
  • Internal documentation updated
  • Create a documentation update request here

@Praveenrajmani Praveenrajmani force-pushed the freeze_until_subsys_init branch from 22eb745 to 94b0a3a Compare May 12, 2023 10:59
@Praveenrajmani
Copy link
Contributor Author

@harshavardhana Can we print the startup message once all the initializations return? Because, with this change, we can't see SQS ARNs as the config wouldn't have been loaded yet.

Can we display the startup message only after all the go-routines return (ie, wg.Wait)

@harshavardhana
Copy link
Member

This is not yet correct @Praveenrajmani let me send an update to this branch.

@harshavardhana harshavardhana force-pushed the freeze_until_subsys_init branch 2 times, most recently from 42d02c0 to 2b6b349 Compare May 17, 2023 22:33
@harshavardhana
Copy link
Member

@Praveenrajmani see what I wanted to do here. Mostly this would work if you can give this a test.

Praveenrajmani and others added 3 commits May 18, 2023 13:24
…letely

Currently, we will miss the events if MinIO server receives the API requests before
the notification sub-system is initialized and the bucket target rules are populated.

This PR will freeze the APIs until these subsystems get completely initialized.
unfreeze after bucket metadata has been loaded.
@Praveenrajmani Praveenrajmani force-pushed the freeze_until_subsys_init branch from 2b6b349 to 54a6d25 Compare May 18, 2023 07:55
@Praveenrajmani
Copy link
Contributor Author

yes, this works @harshavardhana

Copy link
Member

@vadmeste vadmeste left a comment

Choose a reason for hiding this comment

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

LGTM

@harshavardhana harshavardhana merged commit ecfb18b into minio:master May 19, 2023
@poornas
Copy link
Contributor

poornas commented May 19, 2023

LGTM

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.

5 participants