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

indexer binary #168

Merged
merged 36 commits into from
Oct 6, 2022
Merged

indexer binary #168

merged 36 commits into from
Oct 6, 2022

Conversation

miiu96
Copy link
Contributor

@miiu96 miiu96 commented Sep 1, 2022

  • Make indexer a stand-alone binary that will receive data over web-sockets
  • Also the old functionality is there (can be used as a go module)

@iulianpascalau iulianpascalau self-requested a review September 29, 2022 07:15
@bogdan-rosianu bogdan-rosianu self-requested a review September 29, 2022 08:24
@miiu96 miiu96 marked this pull request as ready for review September 29, 2022 08:26
cmd/elasticindexer/config/config.toml Show resolved Hide resolved
cmd/elasticindexer/main.go Outdated Show resolved Hide resolved
factory/wsIndexerFactory.go Outdated Show resolved Hide resolved
factory/wsIndexerFactory.go Show resolved Hide resolved
.github/workflows/pr-build.yml Show resolved Hide resolved
process/wsindexer/indexer.go Show resolved Hide resolved
process/wsindexer/interface.go Show resolved Hide resolved
process/wsindexer/unmarsall.go Outdated Show resolved Hide resolved
process/wsindexer/unmarsall.go Outdated Show resolved Hide resolved
process/wsindexer/unmarsall.go Outdated Show resolved Hide resolved
cmd/elasticindexer/config/config.toml Show resolved Hide resolved
factory/wsIndexerFactory.go Outdated Show resolved Hide resolved
go.mod Outdated
github.com/ElrondNetwork/elrond-go-core v1.1.19-0.20220829095131-68a85ab9fe00
github.com/ElrondNetwork/elrond-go-logger v1.0.7
github.com/ElrondNetwork/elrond-vm-common v1.3.7
github.com/ElrondNetwork/elrond-go-core v1.1.21-0.20220927113554-34ccab6be8c3
Copy link
Contributor

Choose a reason for hiding this comment

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

proper tags before merging

Copy link
Contributor Author

Choose a reason for hiding this comment

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

will do after reviews and testing

cmd/elasticindexer/main.go Outdated Show resolved Hide resolved
cmd/elasticindexer/main.go Show resolved Hide resolved
process/factory/indexerFactory.go Show resolved Hide resolved
process/wsclient/client.go Outdated Show resolved Hide resolved
process/wsclient/client.go Show resolved Hide resolved
process/wsclient/client.go Outdated Show resolved Hide resolved
process/wsindexer/indexer.go Show resolved Hide resolved
url = "http://localhost:9200"
username = ""
password = ""
bulk-request-max-size-in-bytes = 4194304 # 4MB
Copy link
Contributor

Choose a reason for hiding this comment

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

enabled-indices and web-socket are still "prefs".. might move them as well

Copy link
Contributor Author

Choose a reason for hiding this comment

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

moved

log.Info("closing indexer")
if !check.IfNil(fileLogging) {
<-interrupt
wsClient.Close()
Copy link
Contributor

Choose a reason for hiding this comment

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

might add a log before wsClient.Close() closing app at user's signal or something similar

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added

@@ -131,5 +147,15 @@ func initializeLogger(ctx *cli.Context, cfg *config.Config) (file.FileLoggingHan
return nil, err
}

err = logger.RemoveLogObserver(os.Stdout)
Copy link
Contributor

Choose a reason for hiding this comment

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

might extract the new code in a function removeANSICollorsForLogger

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done.

@@ -68,7 +68,7 @@ func TestAccountBalanceNFTTransfer(t *testing.T) {
},
}

err = esProc.SaveTransactions(body, header, pool, coreAlteredAccounts, false, 3)
err = esProc.SaveTransactions(body, header, pool, coreAlteredAccounts, false, testsNumOfShards)
Copy link
Contributor

Choose a reason for hiding this comment

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

testsNumOfShards -> testNumOfShards

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done.

}

func NewWebSocketClient(urlReceive string, actions map[data.OperationType]func(marshalledData []byte) error) (*client, error) {
urlReceiveData := url.URL{Scheme: "ws", Host: fmt.Sprintf(urlReceive), Path: "/operations"}
// New will create a new instance of web-sockets client
Copy link
Contributor

Choose a reason for hiding this comment

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

websocket*

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

if strings.Contains(err.Error(), closedConnection) {
return true
}
log.Warn("websocket error, retrying in ...", "error", err.Error())
Copy link
Contributor

Choose a reason for hiding this comment

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

bad log

Copy link
Contributor Author

Choose a reason for hiding this comment

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

changed

return &indexer{
marshaller: marshaller,
di: dataIndexer,
}, nil
}

func (i *indexer) GetFunctionsMap() map[data.OperationType]func(d []byte) error {
func (i *indexer) GetOperationsMap() map[data.OperationType]func(d []byte) error {
Copy link
Contributor

Choose a reason for hiding this comment

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

add comment

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added

@@ -5,11 +5,13 @@ import (
"github.com/ElrondNetwork/elrond-go-core/data/outport"
)

// WSClient defines what a websockets client should do
Copy link
Contributor

Choose a reason for hiding this comment

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

websocket

Copy link
Contributor Author

Choose a reason for hiding this comment

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

changed

bogdan-rosianu
bogdan-rosianu previously approved these changes Oct 5, 2022
@@ -0,0 +1,16 @@
[config]
enabled-indices = [
Copy link
Contributor

Choose a reason for hiding this comment

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

as agreed, let's move back in the config this list, we will call it available-indices and in this prefs.toml we will have the disabled-indices

return fileLogging, nil
}

func removeANSIColorsForLogger() error {
Copy link
Contributor

Choose a reason for hiding this comment

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

this should be conditioned by a flag. Why do we want to remove the colors?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added a flag for this.

on:
push:
branches:
- master
Copy link
Contributor

Choose a reason for hiding this comment

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

this will need to get changed to branches: [ master, development, feat/*, rc/* ] as fixing a linter issue on a PR will not start the re-check process. Where did you get this file? So we can also apply the patch there.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done.

package integrationtests

const (
//nolint
Copy link
Contributor

Choose a reason for hiding this comment

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

why nolint call here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

gogang-ci linter detects that these constants are not used, which is false


const (
//nolint
testNumOfShards = 3
Copy link
Contributor

Choose a reason for hiding this comment

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

👍 for defining consts

func setLogLevelDebug() {
_ = logger.SetLogLevel("process:DEBUG")
}

//nolint
Copy link
Contributor

Choose a reason for hiding this comment

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

why nolint?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

gogang-ci linter detects that function is not used, which is false

case <-time.After(time.Second):
}
closed := c.listeningOnWebSocket()
if closed {
Copy link
Contributor

Choose a reason for hiding this comment

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

so this process will immediately close if the node process closes but when it is first started, it will wait until the node process boots up. Is this correct?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If it is first started will close immediately, and it will wait on the close when writing something in the Elasticsearch database

}

type argsSaveBlock struct {
TransactionsPool *poolStruct
Copy link
Contributor

Choose a reason for hiding this comment

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

Exported field in an unexported struct of an unexported type. "cessary" exported TransactionsPool param

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. This structure is used to can unmarshal the received data


for _, availableIndex := range availableIndices {
_, found := mapDisabledIndices[availableIndex]
if !found {
Copy link
Contributor

Choose a reason for hiding this comment

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

rename found to shouldSkip and refactor: if shouldSkip { continue } indices = ...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

bogdan-rosianu
bogdan-rosianu previously approved these changes Oct 6, 2022
@miiu96 miiu96 merged commit a09a9b9 into feat/altered-accounts Oct 6, 2022
@miiu96 miiu96 deleted the indexer-binary branch October 6, 2022 12:43
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.

3 participants