Skip to content

tech debt: Indexer initialization improvements #1352

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Dec 7, 2022
Merged

tech debt: Indexer initialization improvements #1352

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7529dfc
Usability and logging improvements.
winder Nov 29, 2022
a28db8c
Add tests, avoid panic to make testable.
winder Nov 30, 2022
cb0a2ec
Switch over one more panic, fix return status.
winder Nov 30, 2022
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
93 changes: 57 additions & 36 deletions cmd/algorand-indexer/daemon.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,8 @@ func DaemonCmd() *cobra.Command {
//Args:
Run: func(cmd *cobra.Command, args []string) {
if err := runDaemon(cfg); err != nil {
panic(exit{1})
fmt.Fprintf(os.Stderr, "Exiting with error: %s\n", err.Error())
os.Exit(1)
Comment on lines -108 to +109
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We weren't printing errors returned from runDaemon!

Panic is no longer needed, if daemon returns then defer was able to run properly.

}
},
}
Expand Down Expand Up @@ -151,34 +152,28 @@ func DaemonCmd() *cobra.Command {
viper.RegisterAlias("algod-net", "algod-address")
viper.RegisterAlias("server", "server-address")
viper.RegisterAlias("token", "api-token")
viper.RegisterAlias("data-dir", "data")
return daemonCmd
}

func configureIndexerDataDir(indexerDataDir string) error {
var err error
if indexerDataDir == "" {
err = fmt.Errorf("indexer data directory was not provided")
logger.WithError(err).Errorf("indexer data directory error, %v", err)
return err
return nil
}
if _, err = os.Stat(indexerDataDir); os.IsNotExist(err) {
err = os.Mkdir(indexerDataDir, 0755)
if err != nil {
logger.WithError(err).Errorf("indexer data directory error, %v", err)
return err
return fmt.Errorf("indexer data directory error, %v", err)
}
}
return err
}

func loadIndexerConfig(indexerDataDir string, configFile string) error {
func resolveConfigFile(indexerDataDir string, configFile string) (string, error) {
var err error
var resolvedConfigPath string
potentialIndexerConfigPath, err := GetConfigFromDataDir(indexerDataDir, autoLoadIndexerConfigFileName, config.FileTypes[:])
if err != nil {
logger.Error(err)
return err
return "", err
}
indexerConfigFound := potentialIndexerConfigPath != ""

Expand All @@ -187,27 +182,31 @@ func loadIndexerConfig(indexerDataDir string, configFile string) error {
if configFile != "" {
err = fmt.Errorf("indexer configuration was found in data directory (%s) as well as supplied via command line. Only provide one",
potentialIndexerConfigPath)
logger.Error(err)
return err
return "", err
}
resolvedConfigPath = potentialIndexerConfigPath
return potentialIndexerConfigPath, nil
} else if configFile != "" {
// user specified
resolvedConfigPath = configFile
} else {
// neither autoload nor user specified
return err
return configFile, nil
}
configs, err := os.Open(resolvedConfigPath)
// neither autoload nor user specified
return "", nil
}

// loadIndexerConfig opens the file and calls viper.ReadConfig
func loadIndexerConfig(configFile string) error {
if configFile == "" {
return nil
}

configs, err := os.Open(configFile)
if err != nil {
logger.WithError(err).Errorf("File Does Not Exist Error: %v", err)
return err
return fmt.Errorf("config file does not exist: %w", err)
}
defer configs.Close()
err = viper.ReadConfig(configs)
if err != nil {
logger.WithError(err).Errorf("invalid config file (%s): %v", viper.ConfigFileUsed(), err)
return err
return fmt.Errorf("invalid config file (%s): %w", configFile, err)
}
return err
}
Expand Down Expand Up @@ -243,33 +242,54 @@ func loadIndexerParamConfig(cfg *daemonConfig) error {

func runDaemon(daemonConfig *daemonConfig) error {
var err error
config.BindFlagSet(daemonConfig.flags)

// check for config environment variables
if daemonConfig.indexerDataDir == "" {
daemonConfig.indexerDataDir = os.Getenv("INDEXER_DATA")
}
if daemonConfig.configFile == "" {
daemonConfig.configFile = os.Getenv("INDEXER_CONFIGFILE")
}
if daemonConfig.algodDataDir == "" {
daemonConfig.algodDataDir = os.Getenv("ALGORAND_DATA")
}

// Create the data directory if necessary/possible
if err = configureIndexerDataDir(daemonConfig.indexerDataDir); err != nil {
return err
}

// Detect the various auto-loading configs from data directory
if err = loadIndexerConfig(daemonConfig.indexerDataDir, daemonConfig.configFile); err != nil {
var configFile string
if configFile, err = resolveConfigFile(daemonConfig.indexerDataDir, daemonConfig.configFile); err != nil {
return err
}

if err = loadIndexerConfig(configFile); err != nil {
return err
}
// We need to re-run this because loading the config file could change these
config.BindFlagSet(daemonConfig.flags)

// Load the Parameter config
if err = loadIndexerParamConfig(daemonConfig); err != nil {
return err
if !daemonConfig.noAlgod && daemonConfig.indexerDataDir == "" {
return fmt.Errorf("indexer data directory was not provided")
}

// Configure the logger after we load all indexer configs
// Configure the logger as soon as we're able so that it can be used.
err = configureLogger()
if err != nil {
fmt.Fprintf(os.Stderr, "failed to configure logger: %v", err)
return err
}

logger.Infof("Using configuration file: %s", viper.ConfigFileUsed())
if configFile != "" {
logger.Infof("Using configuration file: %s", configFile)
}

// Load the Parameter config
if err = loadIndexerParamConfig(daemonConfig); err != nil {
return err
}

if daemonConfig.pidFilePath != "" {
err = iutil.CreateIndexerPidFile(logger, daemonConfig.pidFilePath)
Expand Down Expand Up @@ -300,10 +320,6 @@ func runDaemon(daemonConfig *daemonConfig) error {
defer pprof.StopCPUProfile()
}

if daemonConfig.algodDataDir == "" {
daemonConfig.algodDataDir = os.Getenv("ALGORAND_DATA")
}

ctx, cf := context.WithCancel(context.Background())
defer cf()
{
Expand All @@ -320,7 +336,9 @@ func runDaemon(daemonConfig *daemonConfig) error {

if daemonConfig.algodDataDir != "" {
daemonConfig.algodAddr, daemonConfig.algodToken, _, err = fetcher.AlgodArgsForDataDir(daemonConfig.algodDataDir)
maybeFail(err, "algod data dir err, %v", err)
if err != nil {
return fmt.Errorf("algod data dir err, %v", err)
}
} else if daemonConfig.algodAddr == "" || daemonConfig.algodToken == "" {
// no algod was found
logger.Info("no algod was found, provide either --algod OR --algod-net and --algod-token to enable")
Expand All @@ -341,7 +359,10 @@ func runDaemon(daemonConfig *daemonConfig) error {
opts.AlgodToken = daemonConfig.algodToken
opts.AlgodAddr = daemonConfig.algodAddr

db, availableCh := indexerDbFromFlags(opts)
db, availableCh, err := indexerDbFromFlags(opts)
if err != nil {
return err
}
defer db.Close()
var dataError func() error
if daemonConfig.noAlgod != true {
Expand Down
63 changes: 28 additions & 35 deletions cmd/algorand-indexer/daemon_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,19 +17,10 @@ import (
"github.com/algorand/indexer/util"
)

func createTempDir(t *testing.T) string {
dir, err := os.MkdirTemp("", "indexer")
if err != nil {
t.Fatalf(err.Error())
}
return dir
}

// TestParameterConfigErrorWhenBothFileTypesArePresent test that if both file types are there then it is an error
func TestParameterConfigErrorWhenBothFileTypesArePresent(t *testing.T) {

indexerDataDir := createTempDir(t)
defer os.RemoveAll(indexerDataDir)
indexerDataDir := t.TempDir()
for _, configFiletype := range config.FileTypes {
autoloadPath := filepath.Join(indexerDataDir, autoLoadParameterConfigFileName+"."+configFiletype)
os.WriteFile(autoloadPath, []byte{}, fs.ModePerm)
Expand All @@ -47,8 +38,7 @@ func TestParameterConfigErrorWhenBothFileTypesArePresent(t *testing.T) {
// TestIndexerConfigErrorWhenBothFileTypesArePresent test that if both file types are there then it is an error
func TestIndexerConfigErrorWhenBothFileTypesArePresent(t *testing.T) {

indexerDataDir := createTempDir(t)
defer os.RemoveAll(indexerDataDir)
indexerDataDir := t.TempDir()
for _, configFiletype := range config.FileTypes {
autoloadPath := filepath.Join(indexerDataDir, autoLoadIndexerConfigFileName+"."+configFiletype)
os.WriteFile(autoloadPath, []byte{}, fs.ModePerm)
Expand All @@ -67,8 +57,7 @@ func TestIndexerConfigErrorWhenBothFileTypesArePresent(t *testing.T) {
// enable all parameters are provided together.
func TestConfigWithEnableAllParamsExpectError(t *testing.T) {
for _, configFiletype := range config.FileTypes {
indexerDataDir := createTempDir(t)
defer os.RemoveAll(indexerDataDir)
indexerDataDir := t.TempDir()
autoloadPath := filepath.Join(indexerDataDir, autoLoadIndexerConfigFileName+"."+configFiletype)
os.WriteFile(autoloadPath, []byte{}, fs.ModePerm)
daemonConfig := &daemonConfig{}
Expand All @@ -83,23 +72,21 @@ func TestConfigWithEnableAllParamsExpectError(t *testing.T) {
}

func TestConfigDoesNotExistExpectError(t *testing.T) {
indexerDataDir := createTempDir(t)
defer os.RemoveAll(indexerDataDir)
indexerDataDir := t.TempDir()
tempConfigFile := indexerDataDir + "/indexer.yml"
daemonConfig := &daemonConfig{}
daemonConfig.flags = pflag.NewFlagSet("indexer", 0)
daemonConfig.indexerDataDir = indexerDataDir
daemonConfig.configFile = tempConfigFile
err := runDaemon(daemonConfig)
// This error string is probably OS-specific
errorStr := fmt.Sprintf("open %s: no such file or directory", tempConfigFile)
errorStr := fmt.Sprintf("config file does not exist: open %s: no such file or directory", tempConfigFile)
assert.EqualError(t, err, errorStr)
}

func TestConfigInvalidExpectError(t *testing.T) {
b := bytes.NewBufferString("")
indexerDataDir := createTempDir(t)
defer os.RemoveAll(indexerDataDir)
indexerDataDir := t.TempDir()
tempConfigFile := indexerDataDir + "/indexer-alt.yml"
os.WriteFile(tempConfigFile, []byte(";;;"), fs.ModePerm)
daemonConfig := &daemonConfig{}
Expand All @@ -108,13 +95,12 @@ func TestConfigInvalidExpectError(t *testing.T) {
daemonConfig.configFile = tempConfigFile
logger.SetOutput(b)
err := runDaemon(daemonConfig)
errorStr := "While parsing config: yaml: unmarshal errors:\n line 1: cannot unmarshal !!str `;;;` into map[string]interface {}"
errorStr := fmt.Sprintf("invalid config file (%s): While parsing config: yaml: unmarshal errors:\n line 1: cannot unmarshal !!str `;;;` into map[string]interface {}", tempConfigFile)
assert.EqualError(t, err, errorStr)
}

func TestConfigSpecifiedTwiceExpectError(t *testing.T) {
indexerDataDir := createTempDir(t)
defer os.RemoveAll(indexerDataDir)
indexerDataDir := t.TempDir()
tempConfigFile := indexerDataDir + "/indexer.yml"
os.WriteFile(tempConfigFile, []byte{}, fs.ModePerm)
daemonConfig := &daemonConfig{}
Expand All @@ -130,8 +116,7 @@ func TestConfigSpecifiedTwiceExpectError(t *testing.T) {
func TestLoadAPIConfigGivenAutoLoadAndUserSuppliedExpectError(t *testing.T) {

for _, configFiletype := range config.FileTypes {
indexerDataDir := createTempDir(t)
defer os.RemoveAll(indexerDataDir)
indexerDataDir := t.TempDir()

autoloadPath := filepath.Join(indexerDataDir, autoLoadParameterConfigFileName+"."+configFiletype)
userSuppliedPath := filepath.Join(indexerDataDir, "foobar.yml")
Expand All @@ -148,8 +133,7 @@ func TestLoadAPIConfigGivenAutoLoadAndUserSuppliedExpectError(t *testing.T) {
}

func TestLoadAPIConfigGivenUserSuppliedExpectSuccess(t *testing.T) {
indexerDataDir := createTempDir(t)
defer os.RemoveAll(indexerDataDir)
indexerDataDir := t.TempDir()

userSuppliedPath := filepath.Join(indexerDataDir, "foobar.yml")
cfg := &daemonConfig{}
Expand All @@ -162,8 +146,7 @@ func TestLoadAPIConfigGivenUserSuppliedExpectSuccess(t *testing.T) {

func TestLoadAPIConfigGivenAutoLoadExpectSuccess(t *testing.T) {
for _, configFiletype := range config.FileTypes {
indexerDataDir := createTempDir(t)
defer os.RemoveAll(indexerDataDir)
indexerDataDir := t.TempDir()

autoloadPath := filepath.Join(indexerDataDir, autoLoadParameterConfigFileName+"."+configFiletype)
os.WriteFile(autoloadPath, []byte{}, fs.ModePerm)
Expand All @@ -177,9 +160,7 @@ func TestLoadAPIConfigGivenAutoLoadExpectSuccess(t *testing.T) {
}

func TestIndexerDataDirNotProvidedExpectError(t *testing.T) {
errorStr := "indexer data directory was not provided"

assert.EqualError(t, configureIndexerDataDir(""), errorStr)
assert.NoError(t, configureIndexerDataDir(""))
}

func TestIndexerDataDirCreateFailExpectError(t *testing.T) {
Expand All @@ -189,17 +170,15 @@ func TestIndexerDataDirCreateFailExpectError(t *testing.T) {
}

func TestIndexerPidFileExpectSuccess(t *testing.T) {
indexerDataDir := createTempDir(t)
defer os.RemoveAll(indexerDataDir)
indexerDataDir := t.TempDir()

pidFilePath := path.Join(indexerDataDir, "pidFile")
assert.NoError(t, util.CreateIndexerPidFile(log.New(), pidFilePath))
}

func TestIndexerPidFileCreateFailExpectError(t *testing.T) {
for _, configFiletype := range config.FileTypes {
indexerDataDir := createTempDir(t)
defer os.RemoveAll(indexerDataDir)
indexerDataDir := t.TempDir()
autoloadPath := filepath.Join(indexerDataDir, autoLoadIndexerConfigFileName+"."+configFiletype)
os.WriteFile(autoloadPath, []byte{}, fs.ModePerm)

Expand All @@ -214,3 +193,17 @@ func TestIndexerPidFileCreateFailExpectError(t *testing.T) {
assert.Error(t, util.CreateIndexerPidFile(log.New(), cfg.pidFilePath))
}
}

func TestIndexerMissingDataDir(t *testing.T) {
cfg := &daemonConfig{}
cfg.flags = pflag.NewFlagSet("indexer", 0)
assert.EqualError(t, runDaemon(cfg), "indexer data directory was not provided")
}

func TestOptionalIndexerDataDir(t *testing.T) {
cfg := &daemonConfig{}
cfg.flags = pflag.NewFlagSet("indexer", 0)
cfg.noAlgod = true
// gets to the error beyond the indexer data dir check.
assert.EqualError(t, runDaemon(cfg), "no import db set")
}
6 changes: 5 additions & 1 deletion cmd/algorand-indexer/import.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,11 @@ var importCmd = &cobra.Command{
panic(exit{1})
}

db, availableCh := indexerDbFromFlags(idb.IndexerDbOptions{})
db, availableCh, err := indexerDbFromFlags(idb.IndexerDbOptions{})
if err != nil {
fmt.Fprintf(os.Stderr, err.Error())
return
}
defer db.Close()
<-availableCh

Expand Down
Loading