Skip to content
This repository has been archived by the owner on Nov 24, 2022. It is now read-only.

several commands to help automating miner operation #65

Merged
merged 3 commits into from Oct 14, 2021

Conversation

merlinran
Copy link
Contributor

@merlinran merlinran commented Oct 14, 2021

It does following:

  1. added pause and resume command, to be used in scripts. Tested manually via make up to make sure it doesn't bid when being paused.
  2. added install-service command, to add systemd service. Tested on a Ubuntu 16.04 VPS.
  3. extracted two methods out of the fetch worker for clarity.
  4. a few tiny fixes here and there, will comment inline.

@@ -35,7 +35,7 @@ require (
github.com/stretchr/objx v0.2.0 // indirect
github.com/stretchr/testify v1.7.0
github.com/syndtr/goleveldb v1.0.1-0.20210305035536-64b5b1c73954 // indirect
github.com/textileio/cli v1.0.0
github.com/textileio/cli v1.0.2
Copy link
Contributor Author

Choose a reason for hiding this comment

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

to use the log-plaintext flag

@@ -49,6 +50,8 @@ func createMux(service Service) *http.ServeMux {
mux := http.NewServeMux()
mux.HandleFunc("/health", getOnly(healthHandler))
mux.HandleFunc("/id", getOnly(idHandler(service)))
mux.HandleFunc("/pause", putOnly(pauseHandler(service)))
mux.HandleFunc("/resume", putOnly(resumeHandler(service)))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

figured http PUT is the correct verb.

_ = v.ReadInConfig()
err := v.ReadInConfig()
if err != nil {
log.Errorf("loading config from %s: %v", v.ConfigFileUsed(), err)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

print error when config file format is invalid as a result of manual editing. currently the log message is very confusing, something like 2021/10/13 21:14:39 --storage-provider-id is required. See ‘bidbot help init’ for instructions.

don't use cli.CheckErrf because we need to allow bidbot init to run, at which point the config file doesn't exist.

Restart=on-failure
RestartSec=10

ExecStart=%s daemon --log-plaintext
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Even with this, syslog on my Ubuntu16 still replaces tab with #011 which makes the output messy, but at least we tried. maybe later versions don't have the problem?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not entirely sure about the problem, but found this. Dunno if worth fixing, just sharing 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.

sorry I should've mentioned I checked this exact SO thread. Just don't think we should mess with rsyslog.conf.

RestartSec=10

ExecStart=%s daemon --log-plaintext
StandardOutput=journal
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 tried using append:/var/log/bidbot.log, but Ubuntu16 doesn't recognize it - too old a systemctl version, so I resorted to this. Anyway, the user can configure syslog conf to divert bidbot log to a separate file if they want. Also this is meant to be a start point to which the miner can adjust by themselves.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, totally agree. Miners are "operators" by nature, probably they're expert in systemd configs and will tune it to fit their architecture of logging and similars.

_, err = user.Lookup(u)
cli.CheckErrf("looking up user: %v", err)
_, err = user.LookupGroup(g)
cli.CheckErrf("looking up group: %v", err)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

make sure the user and group exists. thought about just use the current user, but feels too clever.

Copy link
Contributor

Choose a reason for hiding this comment

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

Nice, looks good to me.

@@ -571,20 +571,6 @@ func (s *Store) enqueueDataURI(txn ds.Txn, b *Bid) error {
func (s *Store) fetchWorker(num int) {
defer s.wg.Done()

fail := func(b *Bid, err error) (status BidStatus) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

extracted methods to avoid fetchWorker becoming unmanageable.

@merlinran merlinran requested a review from jsign October 14, 2021 18:41
Signed-off-by: Merlin Ran <merlinran@gmail.com>
Signed-off-by: Merlin Ran <merlinran@gmail.com>
Copy link
Contributor

@jsign jsign left a comment

Choose a reason for hiding this comment

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

🚀

main.go Outdated
Comment on lines 615 to 617
if runtime.GOOS != "linux" {
log.Fatal("only available in Linux")
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we should add that we're assuming systemd?
Is definitely the safest bet we can do, but I think other distros might not have it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good point. will add both here and the command description.

main.go Outdated Show resolved Hide resolved
_, err = user.Lookup(u)
cli.CheckErrf("looking up user: %v", err)
_, err = user.LookupGroup(g)
cli.CheckErrf("looking up group: %v", err)
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice, looks good to me.

Restart=on-failure
RestartSec=10

ExecStart=%s daemon --log-plaintext
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not entirely sure about the problem, but found this. Dunno if worth fixing, just sharing here :)

RestartSec=10

ExecStart=%s daemon --log-plaintext
StandardOutput=journal
Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, totally agree. Miners are "operators" by nature, probably they're expert in systemd configs and will tune it to fit their architecture of logging and similars.

Restart=on-failure
RestartSec=10

ExecStart=%s daemon --log-plaintext
Copy link
Contributor

Choose a reason for hiding this comment

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

So bidbot should be on PATH on the executing user?
(Kind of an open question to see if that might be relevant to clarify somewhere)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This will be the full path of bidbot extracted from os.Executable(), so no PATH is involved here.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh right, is the template. Dismiss

@@ -154,6 +155,7 @@ type Service struct {
lc lotusclient.LotusClient
store *bidstore.Store

paused int32 // atomic. 1 == paused; 0 == not paused
Copy link
Contributor

Choose a reason for hiding this comment

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

You definitely like atomics :P

Copy link
Contributor Author

Choose a reason for hiding this comment

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

bad habit formed when frequently need to have thousands of checks per second, making mutex too expensive. it saves a few lines of code too, so I'm kinda still like it.

// AuctionsHandler implements MessageHandler.
func (s *Service) AuctionsHandler(from core.ID, a *pb.Auction) error {
if s.isPaused() {
log.Info("not bidding when bidbot is paused")
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice, this will be useful for sure.

Signed-off-by: Merlin Ran <merlinran@gmail.com>
@merlinran merlinran merged commit 38abb3c into main Oct 14, 2021
@merlinran merlinran deleted the merlin/automation-commands branch October 14, 2021 20:34
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants