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

api: support to query whether pd has loaded region #8749

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 25 additions & 8 deletions pkg/storage/storage.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,9 +77,10 @@ type coreStorage struct {
Storage
regionStorage endpoint.RegionStorage

useRegionStorage int32
regionLoaded bool
mu syncutil.Mutex
useRegionStorage int32
Copy link
Contributor

Choose a reason for hiding this comment

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

Is that safe to switch between using or not using RegionStore in the master branch or the latest releases now?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

defaultUseRegionStorage is true and only check switch when it was just elected leader.

regionLoadedFromDefault bool
regionLoadedFromStorage bool
Copy link
Member

Choose a reason for hiding this comment

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

Why do we need to distinguish these two kinds?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I want to avoid problems after ps.useRegionStorage switch

Copy link
Member

Choose a reason for hiding this comment

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

How about using string?

regionLoadedFrom: "", "etcd", "leveldb"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think so, we just have these two ways now

Copy link
Member

Choose a reason for hiding this comment

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

I prefer just using a single field to store this info like:

type regionSource int

const (
	unloaded regionSource = iota
	etcd
	leveldb
)

mu syncutil.RWMutex
}

// NewCoreStorage creates a new core storage with the given default and region storage.
Expand Down Expand Up @@ -133,17 +134,22 @@ func TryLoadRegionsOnce(ctx context.Context, s Storage, f func(region *core.Regi
return s.LoadRegions(ctx, f)
}

ps.mu.Lock()
defer ps.mu.Unlock()

if atomic.LoadInt32(&ps.useRegionStorage) == 0 {
return ps.Storage.LoadRegions(ctx, f)
err := ps.Storage.LoadRegions(ctx, f)
if err == nil {
ps.regionLoadedFromDefault = true
}
return err
}

ps.mu.Lock()
defer ps.mu.Unlock()
if !ps.regionLoaded {
if !ps.regionLoadedFromStorage {
if err := ps.regionStorage.LoadRegions(ctx, f); err != nil {
return err
}
ps.regionLoaded = true
ps.regionLoadedFromStorage = true
}
return nil
Copy link
Member

Choose a reason for hiding this comment

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

Seems we can use *coreStorage.LoadRegions() directly?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It doesn't check ps.regionLoadedFromStorage

}
Expand Down Expand Up @@ -197,3 +203,14 @@ func (ps *coreStorage) Close() error {
}
return nil
}

// AreRegionsLoaded returns whether the regions are loaded.
func AreRegionsLoaded(s Storage) bool {
ps := s.(*coreStorage)
ps.mu.RLock()
defer ps.mu.RUnlock()
if atomic.LoadInt32(&ps.useRegionStorage) == 0 {
return ps.regionLoadedFromDefault
}
return ps.regionLoadedFromStorage
}
1 change: 1 addition & 0 deletions pkg/versioninfo/versioninfo.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ type Status struct {
Version string `json:"version"`
GitHash string `json:"git_hash"`
StartTimestamp int64 `json:"start_timestamp"`
Loaded bool `json:"loaded"`
Copy link
Member

Choose a reason for hiding this comment

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

Prefer RegionLoaded

}

const (
Expand Down
2 changes: 2 additions & 0 deletions server/api/status.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ package api
import (
"net/http"

"github.com/tikv/pd/pkg/storage"
"github.com/tikv/pd/pkg/versioninfo"
"github.com/tikv/pd/server"
"github.com/unrolled/render"
Expand Down Expand Up @@ -44,6 +45,7 @@ func (h *statusHandler) GetPDStatus(w http.ResponseWriter, _ *http.Request) {
GitHash: versioninfo.PDGitHash,
Version: versioninfo.PDReleaseVersion,
StartTimestamp: h.svr.StartTimestamp(),
Loaded: storage.AreRegionsLoaded(h.svr.GetStorage()),
}

h.rd.JSON(w, http.StatusOK, version)
Expand Down
Loading