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.

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

Cleanup: Remove indexer v1 from codebase #5477

Merged
merged 3 commits into from
Jun 21, 2023
Merged

Cleanup: Remove indexer v1 from codebase #5477

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
ff87e54
Remove indexer v1 from go-algorand codebase.
gmalouf Jun 16, 2023
5f766d0
Removes isIndexerActive config property from local template and gener…
gmalouf Jun 16, 2023
02644a8
Update cmd/goal/node.go
gmalouf Jun 16, 2023
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
2 changes: 2 additions & 0 deletions .golangci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ linters-settings:
exclude-functions:
# We do this 121 times and never check the error.
- (*github.com/spf13/cobra.Command).MarkFlagRequired
- (*github.com/spf13/pflag.FlagSet).MarkDeprecated
- (*github.com/spf13/pflag.FlagSet).MarkShorthandDeprecated
govet:
check-shadowing: true
settings:
Expand Down
12 changes: 8 additions & 4 deletions cmd/goal/node.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,6 @@ var waitSec uint32
var newNodeNetwork string
var newNodeDestination string
var newNodeArchival bool
var newNodeIndexer bool
var newNodeRelay string
var newNodeFullConfig bool
var watchMillisecond uint64
Expand Down Expand Up @@ -98,7 +97,13 @@ func init() {
createCmd.Flags().StringVar(&newNodeDestination, "destination", "", "Destination path for the new node")
createCmd.Flags().BoolVarP(&newNodeArchival, "archival", "a", localDefaults.Archival, "Make the new node archival, storing all blocks")
createCmd.Flags().BoolVarP(&runUnderHost, "hosted", "H", localDefaults.RunHosted, "Configure the new node to run hosted by algoh")
createCmd.Flags().BoolVarP(&newNodeIndexer, "indexer", "i", localDefaults.IsIndexerActive, "Configure the new node to enable the indexer feature (implies --archival)")

// The flag for enabling an internal indexer is now deprecated, but we keep it for backwards compatibility for now.
indexerFlagName := "indexer"
_ = createCmd.Flags().BoolP(indexerFlagName, "i", false, "")
createCmd.Flags().MarkDeprecated(indexerFlagName, "no longer used, please remove from your scripts")
createCmd.Flags().MarkShorthandDeprecated(indexerFlagName, "no longer used, please remove from your scripts")

createCmd.Flags().StringVar(&newNodeRelay, "relay", localDefaults.NetAddress, "Configure as a relay with specified listening address (NetAddress)")
createCmd.Flags().StringVar(&listenIP, "api", "", "REST API Endpoint")
createCmd.Flags().BoolVar(&newNodeFullConfig, "full-config", false, "Store full config file")
Expand Down Expand Up @@ -680,8 +685,7 @@ var createCmd = &cobra.Command{
reportErrorf(errorNodeCreationIPFailure, listenIP)
}
}
localConfig.Archival = newNodeArchival || newNodeRelay != "" || newNodeIndexer
localConfig.IsIndexerActive = newNodeIndexer
localConfig.Archival = newNodeArchival || newNodeRelay != ""
localConfig.RunHosted = runUnderHost
localConfig.EnableLedgerService = localConfig.Archival
localConfig.EnableBlockService = localConfig.Archival
Expand Down
4 changes: 0 additions & 4 deletions config/localTemplate.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -237,10 +237,6 @@ type Local struct {
// the max size the sync server would return
TxSyncServeResponseSize int `version[3]:"1000000"`

// IsIndexerActive indicates whether to activate the indexer for fast retrieval of transactions
// Note -- Indexer cannot operate on non Archival nodes
IsIndexerActive bool `version[3]:"false"`
Copy link
Contributor

@winder winder Jun 16, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Need to leave this in or else we'd config file parsing failures. Even though most configs will have it set to false already, I'm pretty sure unknown fields are illegal.

Copy link
Contributor Author

@gmalouf gmalouf Jun 16, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I thought that too, but check out the config test - it's loading config versions prior to v28 that have the property without error.

Copy link
Contributor

Choose a reason for hiding this comment

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

Woah. I wonder if its always been that way. Thanks for the correction!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

https://github.com/algorand/go-algorand/blob/master/config/config_test.go#L292

Copy link
Contributor

Choose a reason for hiding this comment

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

// To unmarshal JSON into a struct, Unmarshal matches incoming object
// keys to the keys used by Marshal (either the struct field name or its tag),
// preferring an exact match but also accepting a case-insensitive match. By
// default, object keys which don't have a corresponding struct field are
// ignored (see Decoder.DisallowUnknownFields for an alternative).

indeed, what a surprize, I thought it fails like msgp

Copy link
Contributor

Choose a reason for hiding this comment

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

I checked IsIndexerActive=true also deserialized without problems and matches the documentation.

Not sure tho if silently removal config values is a good practice? Maybe there should be some kind of deprecated fields and warning in node.log?

Copy link
Contributor

Choose a reason for hiding this comment

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

It would be cool if our config tagging system supported something like

IsIndexerActive bool `version[3]:"false" version[28]:DEPRECATED`

Copy link
Contributor

Choose a reason for hiding this comment

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

so you could get deprecation warnings logged at startup time


// UseXForwardedForAddress indicates whether or not the node should use the X-Forwarded-For HTTP Header when
// determining the source of a connection. If used, it should be set to the string "X-Forwarded-For", unless the
// proxy vendor provides another header field. In the case of CloudFlare proxy, the "CF-Connecting-IP" header
Expand Down
1 change: 0 additions & 1 deletion config/local_defaults.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,6 @@ var defaultLocal = Local{
IncomingConnectionsLimit: 2400,
IncomingMessageFilterBucketCount: 5,
IncomingMessageFilterBucketSize: 512,
IsIndexerActive: false,
LedgerSynchronousMode: 2,
LogArchiveMaxAge: "",
LogArchiveName: "node.archive.log",
Expand Down
5 changes: 0 additions & 5 deletions daemon/algod/api/server/v2/test/helpers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ import (
"github.com/algorand/go-algorand/ledger/simulation"
"github.com/algorand/go-algorand/logging"
"github.com/algorand/go-algorand/node"
"github.com/algorand/go-algorand/node/indexer"
"github.com/algorand/go-algorand/protocol"
"github.com/algorand/go-algorand/util/db"
)
Expand Down Expand Up @@ -221,10 +220,6 @@ func (m *mockNode) Uint64() uint64 {
return 1
}

func (m *mockNode) Indexer() (*indexer.Indexer, error) {
return nil, fmt.Errorf("indexer not implemented")
}

func (m *mockNode) GetTransactionByID(txid transactions.Txid, rnd basics.Round) (node.TxnWithStatus, error) {
return node.TxnWithStatus{}, fmt.Errorf("get transaction by id not implemented")
}
Expand Down
1 change: 0 additions & 1 deletion installer/config.json.example
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,6 @@
"IncomingConnectionsLimit": 2400,
"IncomingMessageFilterBucketCount": 5,
"IncomingMessageFilterBucketSize": 512,
"IsIndexerActive": false,
"LedgerSynchronousMode": 2,
"LogArchiveMaxAge": "",
"LogArchiveName": "node.archive.log",
Expand Down
279 changes: 0 additions & 279 deletions node/indexer/db.go
View file
Open in desktop

This file was deleted.

Loading