Skip to content

Commit

Permalink
feat: parallel fetching of component types to improve performance
Browse files Browse the repository at this point in the history
Fix redhat-developer#3105 as we now also fetch devfiles for completion, which was
previously prohibitively slow.
  • Loading branch information
metacosm committed May 11, 2020
1 parent bfa9922 commit 5ddf83b
Show file tree
Hide file tree
Showing 4 changed files with 124 additions and 93 deletions.
143 changes: 67 additions & 76 deletions pkg/catalog/catalog.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,44 +5,32 @@ import (
"fmt"
"sort"
"strings"
"sync"

imagev1 "github.com/openshift/api/image/v1"
"github.com/openshift/odo/pkg/log"
"github.com/openshift/odo/pkg/occlient"
"github.com/openshift/odo/pkg/preference"
"github.com/openshift/odo/pkg/util"
"github.com/pkg/errors"
"gopkg.in/yaml.v2"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/klog"
)

// GetDevfileRegistries gets devfile registries from preference file,
// if registry name is specified return the specific registry, otherwise return all registries
func GetDevfileRegistries(registryName string) (map[string]string, error) {
devfileRegistries := make(map[string]string)
// DevfileRegistries contains the links of all devfile registries
var DevfileRegistries = []string{
"https://raw.githubusercontent.com/elsony/devfile-registry/master",
"https://che-devfile-registry.openshift.io/",
}

cfg, err := preference.New()
if err != nil {
return nil, err
}
func getDevfileIndexWith(link string, entries *[]DevfileIndexEntry, err error, wg *sync.WaitGroup) {
defer wg.Done()

if cfg.OdoSettings.RegistryList != nil {
for _, registry := range *cfg.OdoSettings.RegistryList {
if len(registryName) != 0 {
if registryName == registry.Name {
devfileRegistries[registry.Name] = registry.URL
return devfileRegistries, nil
}
} else {
devfileRegistries[registry.Name] = registry.URL
}
}
} else {
return nil, nil
devfileIndexLink := link + "/devfiles/index.json"
indexEntries, err := GetDevfileIndex(devfileIndexLink)
for i := range indexEntries {
indexEntries[i].Links.base = link
}

return devfileRegistries, nil
*entries = append(*entries, indexEntries...)
}

// GetDevfileIndex loads the devfile registry index.json
Expand All @@ -62,6 +50,28 @@ func GetDevfileIndex(devfileIndexLink string) ([]DevfileIndexEntry, error) {
return devfileIndex, nil
}

var mutex = &sync.Mutex{}

func getDevfileWith(link string, devfileIndexEntry DevfileIndexEntry, catalogDevfileList *DevfileComponentTypeList, err error, wg *sync.WaitGroup) {
defer wg.Done()

devfile, err := GetDevfile(link)

// Populate devfile component with devfile data and form devfile component list
catalogDevfile := DevfileComponentType{
Name: strings.TrimSuffix(devfile.MetaData.GenerateName, "-"),
DisplayName: devfileIndexEntry.DisplayName,
Description: devfileIndexEntry.Description,
Link: devfileIndexEntry.Links.Link,
Support: IsDevfileComponentSupported(devfile),
Registry: devfileIndexEntry.Links.base,
}

mutex.Lock()
catalogDevfileList.Items = append(catalogDevfileList.Items, catalogDevfile)
mutex.Unlock()
}

// GetDevfile loads the devfile
func GetDevfile(devfileLink string) (Devfile, error) {
var devfile Devfile
Expand Down Expand Up @@ -127,64 +137,45 @@ func IsDevfileComponentSupported(devfile Devfile) bool {
}

// ListDevfileComponents lists all the available devfile components
func ListDevfileComponents(registryName string) (DevfileComponentTypeList, error) {
var catalogDevfileList DevfileComponentTypeList
var err error
func ListDevfileComponents() (DevfileComponentTypeList, error) {
catalogDevfileList := &DevfileComponentTypeList{}
catalogDevfileList.DevfileRegistries = DevfileRegistries

// Get devfile registries
catalogDevfileList.DevfileRegistries, err = GetDevfileRegistries(registryName)
if err != nil {
return catalogDevfileList, err
}
if catalogDevfileList.DevfileRegistries == nil {
return catalogDevfileList, nil
}

for registryName, registryURL := range catalogDevfileList.DevfileRegistries {
devfileIndex := make([]DevfileIndexEntry, 0, 20)
var registryWG sync.WaitGroup
for _, devfileRegistry := range DevfileRegistries {
// Load the devfile registry index.json
devfileIndexLink := registryURL + "/devfiles/index.json"
devfileIndex, err := GetDevfileIndex(devfileIndexLink)
registryWG.Add(1)
var err error
go getDevfileIndexWith(devfileRegistry, &devfileIndex, err, &registryWG)
if err != nil {
log.Warningf("Registry %s is not set up properly with error: %v", registryName, err)
break
return DevfileComponentTypeList{}, err
}

// 1. Load devfiles that indexed in devfile registry index.json
// 2. Populate devfile components with devfile data
// 3. Form devfile component list
for _, devfileIndexEntry := range devfileIndex {
devfileIndexEntryLink := devfileIndexEntry.Links.Link

// Load the devfile
devfileLink := registryURL + devfileIndexEntryLink
// TODO: We send http get resquest in this function multiple times
// since devfile registry uses different links to host different devfiles,
// this can reduce the performance especially when we load devfiles from
// big registry. We may need to rethink and optimize this in the future
devfile, err := GetDevfile(devfileLink)
if err != nil {
log.Warningf("Registry %s is not set up properly with error: %v", registryName, err)
break
}

// Populate devfile component with devfile data and form devfile component list
catalogDevfile := DevfileComponentType{
Name: strings.TrimSuffix(devfile.MetaData.GenerateName, "-"),
DisplayName: devfileIndexEntry.DisplayName,
Description: devfileIndexEntry.Description,
Link: devfileIndexEntryLink,
Support: IsDevfileComponentSupported(devfile),
Registry: Registry{
Name: registryName,
URL: registryURL,
},
}

catalogDevfileList.Items = append(catalogDevfileList.Items, catalogDevfile)
}
registryWG.Wait()

// 1. Load devfiles that indexed in devfile registry index.json
// 2. Populate devfile components with devfile data
// 3. Form devfile component list
var wg sync.WaitGroup
for _, devfileIndexEntry := range devfileIndex {
devfileLink := devfileIndexEntry.Links.base + devfileIndexEntry.Links.Link

// Load the devfile
// TODO: We send http get resquest in this function mutiple times
// since devfile registry uses different links to host different devfiles,
// this can reduce the performance especially when we load devfiles from
// big registry. We may need to rethink and optimize this in the future
wg.Add(1)
var err error
go getDevfileWith(devfileLink, devfileIndexEntry, catalogDevfileList, err, &wg)
if err != nil {
return DevfileComponentTypeList{}, err
}
}
wg.Wait()

return catalogDevfileList, nil
return *catalogDevfileList, nil
}

// ListComponents lists all the available component types
Expand Down
1 change: 1 addition & 0 deletions pkg/catalog/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ type DevfileIndexEntry struct {
Icon string `json:"icon"`
GlobalMemoryLimit string `json:"globalMemoryLimit"`
Links struct {
base string
Link string `json:"self"`
} `json:"links"`
}
Expand Down
20 changes: 18 additions & 2 deletions pkg/odo/cli/catalog/list/components.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"io"
"os"
"strings"
"sync"
"text/tabwriter"

"github.com/openshift/odo/pkg/catalog"
Expand Down Expand Up @@ -42,9 +43,17 @@ func NewListComponentsOptions() *ListComponentsOptions {

// Complete completes ListComponentsOptions after they've been created
func (o *ListComponentsOptions) Complete(name string, cmd *cobra.Command, args []string) (err error) {
var wg sync.WaitGroup

if !pushtarget.IsPushTargetDocker() {
o.Context = genericclioptions.NewContext(cmd)
o.catalogList, err = catalog.ListComponents(o.Client)

wg.Add(1)
var err error
go func(err error) {
defer wg.Done()
o.catalogList, err = catalog.ListComponents(o.Client)
}(err)
if err != nil {
if experimental.IsExperimentalModeEnabled() {
klog.V(4).Info("Please log in to an OpenShift cluster to list OpenShift/s2i components")
Expand All @@ -57,7 +66,12 @@ func (o *ListComponentsOptions) Complete(name string, cmd *cobra.Command, args [
}

if experimental.IsExperimentalModeEnabled() {
o.catalogDevfileList, err = catalog.ListDevfileComponents("")
wg.Add(1)
var err error
go func(err error) {
defer wg.Done()
o.catalogDevfileList, err = catalog.ListDevfileComponents("")
}(err)
if err != nil {
return err
}
Expand All @@ -67,6 +81,8 @@ func (o *ListComponentsOptions) Complete(name string, cmd *cobra.Command, args [
}
}

wg.Wait()

return
}

Expand Down
53 changes: 38 additions & 15 deletions pkg/odo/util/completion/completionhandlers.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,22 +2,21 @@ package completion

import (
"fmt"
"strings"

"github.com/openshift/odo/pkg/config"

appsv1 "github.com/openshift/api/apps/v1"
"github.com/openshift/odo/pkg/application"
"github.com/openshift/odo/pkg/catalog"
"github.com/openshift/odo/pkg/component"
componentlabels "github.com/openshift/odo/pkg/component/labels"
"github.com/openshift/odo/pkg/config"
"github.com/openshift/odo/pkg/odo/genericclioptions"
"github.com/openshift/odo/pkg/service"
"github.com/openshift/odo/pkg/storage"
"github.com/openshift/odo/pkg/url"
"github.com/openshift/odo/pkg/util"
"github.com/posener/complete"
"github.com/spf13/cobra"
"strings"
"sync"
)

// ServiceCompletionHandler provides service name completion for the current project and application
Expand Down Expand Up @@ -271,20 +270,44 @@ var StorageUnMountCompletionHandler = func(cmd *cobra.Command, args parsedArgs,
// CreateCompletionHandler provides component type completion in odo create command
var CreateCompletionHandler = func(cmd *cobra.Command, args parsedArgs, context *genericclioptions.Context) (completions []string) {
completions = make([]string, 0)
catalogList, err := catalog.ListComponents(context.Client)
if err != nil {
return completions
}
found := false

for _, builder := range catalogList.Items {
// we found the builder name in the list which means
// that the builder name has been already selected by the user so no need to suggest more
if args.commands[builder.Name] {
return nil
var wg sync.WaitGroup
wg.Add(2)

go func(completions *[]string, wg *sync.WaitGroup) {
defer wg.Done()

catalogList, _ := catalog.ListComponents(context.Client)
for _, builder := range catalogList.Items {
if args.commands[builder.Name] {
found = true
return
}
if len(builder.Spec.NonHiddenTags) > 0 {
*completions = append(*completions, builder.Name)
}
}
completions = append(completions, builder.Name)
}
}(&completions, &wg)

go func(completions *[]string, wg *sync.WaitGroup) {
defer wg.Done()

components, _ := catalog.ListDevfileComponents()
for _, devfile := range components.Items {
if args.commands[devfile.Name] {
found = true
return
}
*completions = append(*completions, devfile.Name)
}
}(&completions, &wg)

wg.Wait()

if found {
return nil
}
return completions
}

Expand Down

0 comments on commit 5ddf83b

Please sign in to comment.