-
Notifications
You must be signed in to change notification settings - Fork 1
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
Test main loop #6
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,7 +3,6 @@ package ingester | |
import ( | ||
"context" | ||
"log/slog" | ||
"sync" | ||
"time" | ||
|
||
"github.com/duneanalytics/blockchain-ingester/client/duneapi" | ||
|
@@ -12,7 +11,8 @@ import ( | |
) | ||
|
||
type Ingester interface { | ||
Run(ctx context.Context, wg *sync.WaitGroup) error | ||
// Run starts the ingester and blocks until the context is cancelled or maxCount blocks are ingested | ||
Run(ctx context.Context, startBlockNumber, maxCount int64) error | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
|
||
// ConsumeBlocks sends blocks from startBlockNumber to endBlockNumber to outChan, inclusive. | ||
// If endBlockNumber is -1, it sends blocks from startBlockNumber to the tip of the chain | ||
|
@@ -33,9 +33,8 @@ type Ingester interface { | |
const defaultMaxBatchSize = 1 | ||
|
||
type Config struct { | ||
MaxBatchSize int | ||
StartBlockHeight int64 | ||
PollInterval time.Duration | ||
MaxBatchSize int | ||
PollInterval time.Duration | ||
} | ||
|
||
type Info struct { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,7 +3,6 @@ package ingester | |
import ( | ||
"context" | ||
"fmt" | ||
"sync" | ||
"sync/atomic" | ||
"time" | ||
|
||
|
@@ -12,30 +11,28 @@ import ( | |
"golang.org/x/sync/errgroup" | ||
) | ||
|
||
func (i *ingester) Run(ctx context.Context, wg *sync.WaitGroup) error { | ||
defer wg.Done() | ||
|
||
func (i *ingester) Run(ctx context.Context, startBlockNumber, maxCount int64) error { | ||
inFlightChan := make(chan models.RPCBlock, i.cfg.MaxBatchSize) | ||
defer close(inFlightChan) | ||
|
||
var err error | ||
|
||
startBlockNumber := i.cfg.StartBlockHeight | ||
if startBlockNumber <= 0 { | ||
if startBlockNumber < 0 { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good! We do indeed want block 0 |
||
startBlockNumber, err = i.node.LatestBlockNumber() | ||
if err != nil { | ||
return errors.Errorf("failed to get latest block number: %w", err) | ||
} | ||
} | ||
|
||
i.log.Info("Starting ingester", | ||
"maxBatchSize", i.cfg.MaxBatchSize, | ||
"startBlockHeight", i.cfg.StartBlockHeight, | ||
"startBlockNumber", startBlockNumber, | ||
"maxCount", maxCount, | ||
) | ||
|
||
errGroup, ctx := errgroup.WithContext(ctx) | ||
errGroup.Go(func() error { | ||
return i.ConsumeBlocks(ctx, inFlightChan, startBlockNumber, -1) | ||
return i.ConsumeBlocks(ctx, inFlightChan, startBlockNumber, startBlockNumber+maxCount) | ||
}) | ||
errGroup.Go(func() error { | ||
return i.SendBlocks(ctx, inFlightChan) | ||
|
@@ -44,9 +41,14 @@ func (i *ingester) Run(ctx context.Context, wg *sync.WaitGroup) error { | |
return i.ReportProgress(ctx) | ||
}) | ||
|
||
return errGroup.Wait() | ||
if err := errGroup.Wait(); err != nil && err != ErrFinishedConsumeBlocks { | ||
return err | ||
} | ||
return nil | ||
} | ||
|
||
var ErrFinishedConsumeBlocks = errors.New("finished ConsumeBlocks") | ||
|
||
// ConsumeBlocks from the NPC Node | ||
func (i *ingester) ConsumeBlocks( | ||
ctx context.Context, outChan chan models.RPCBlock, startBlockNumber, endBlockNumber int64, | ||
|
@@ -55,6 +57,7 @@ func (i *ingester) ConsumeBlocks( | |
latestBlockNumber := i.tryUpdateLatestBlockNumber() | ||
|
||
waitForBlock := func(blockNumber, latestBlockNumber int64) int64 { | ||
// TODO: handle cancellation here | ||
for blockNumber > latestBlockNumber { | ||
i.log.Info(fmt.Sprintf("Waiting %v for block to be available..", i.cfg.PollInterval), | ||
"blockNumber", blockNumber, | ||
|
@@ -66,7 +69,7 @@ func (i *ingester) ConsumeBlocks( | |
return latestBlockNumber | ||
} | ||
|
||
for blockNumber := startBlockNumber; dontStop || startBlockNumber <= endBlockNumber; blockNumber++ { | ||
for blockNumber := startBlockNumber; dontStop || blockNumber <= endBlockNumber; blockNumber++ { | ||
|
||
latestBlockNumber = waitForBlock(blockNumber, latestBlockNumber) | ||
startTime := time.Now() | ||
|
@@ -92,7 +95,7 @@ func (i *ingester) ConsumeBlocks( | |
|
||
select { | ||
case <-ctx.Done(): | ||
return nil | ||
return ctx.Err() | ||
case outChan <- block: | ||
} | ||
|
||
|
@@ -108,7 +111,7 @@ func (i *ingester) ConsumeBlocks( | |
) | ||
} | ||
} | ||
return nil | ||
return ErrFinishedConsumeBlocks // FIXME: this is wrong | ||
} | ||
|
||
func (i *ingester) SendBlocks(ctx context.Context, blocksCh <-chan models.RPCBlock) error { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,15 +1,180 @@ | ||
package ingester_test | ||
|
||
import "testing" | ||
import ( | ||
"context" | ||
"io" | ||
"log/slog" | ||
"sync/atomic" | ||
"testing" | ||
"time" | ||
|
||
func TestBlockConsumptionLoop(t *testing.T) { | ||
t.Skip("not implemented") | ||
"github.com/duneanalytics/blockchain-ingester/ingester" | ||
duneapi_mock "github.com/duneanalytics/blockchain-ingester/mocks/duneapi" | ||
jsonrpc_mock "github.com/duneanalytics/blockchain-ingester/mocks/jsonrpc" | ||
"github.com/duneanalytics/blockchain-ingester/models" | ||
"github.com/go-errors/errors" | ||
"github.com/stretchr/testify/require" | ||
) | ||
|
||
func TestBlockConsumptionLoopErrors(t *testing.T) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's not immediately clear to me what this test is testing. Is it testing that we exit the function cleanly under these scenarios + what blocks have been ingested? Or am I misunderstanding? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this test is bogus right now, because I realized (while trying to get this test any decent) that the logic I created is just bad... The problem is:
in short, this test just shows we're a bit confused on what to expect from the test. |
||
testcases := []struct { | ||
name string | ||
LatestIsBroken bool | ||
BlockByNumberIsBroken bool | ||
}{ | ||
{ | ||
name: "we're up to date, following the head", | ||
LatestIsBroken: false, | ||
BlockByNumberIsBroken: false, | ||
}, | ||
{ | ||
name: "the RPC node is broken, all API calls are failing", | ||
LatestIsBroken: true, | ||
BlockByNumberIsBroken: true, | ||
}, | ||
{ | ||
name: "BlockByNumber, a specific jsonRPC on the RPC node is broken", | ||
LatestIsBroken: false, | ||
BlockByNumberIsBroken: true, | ||
}, | ||
} | ||
|
||
for _, tc := range testcases { | ||
t.Run(tc.name, func(t *testing.T) { | ||
if tc.LatestIsBroken { | ||
t.Skip("latest block number is broken, we don't behave correctly yet") | ||
} | ||
ctx, cancel := context.WithCancel(context.Background()) | ||
maxBlockNumber := int64(100) | ||
producedBlockNumber := int64(0) | ||
rpcClient := &jsonrpc_mock.BlockchainClientMock{ | ||
LatestBlockNumberFunc: func() (int64, error) { | ||
if tc.LatestIsBroken { | ||
return 0, errors.New("latest block number is broken") | ||
} | ||
return maxBlockNumber, nil | ||
}, | ||
BlockByNumberFunc: func(_ context.Context, blockNumber int64) (models.RPCBlock, error) { | ||
if tc.BlockByNumberIsBroken { | ||
return models.RPCBlock{}, errors.New("block by number is broken") | ||
} | ||
if blockNumber > maxBlockNumber { | ||
// end tests | ||
cancel() | ||
} | ||
atomic.StoreInt64(&producedBlockNumber, blockNumber) | ||
return models.RPCBlock{ | ||
BlockNumber: blockNumber, | ||
Payload: []byte(`block`), | ||
}, nil | ||
}, | ||
} | ||
ing := ingester.New(slog.New(slog.NewTextHandler(io.Discard, nil)), rpcClient, nil, ingester.Config{ | ||
MaxBatchSize: 1, | ||
PollInterval: 1000 * time.Millisecond, | ||
}) | ||
|
||
outCh := make(chan models.RPCBlock, maxBlockNumber+1) | ||
defer close(outCh) | ||
err := ing.ConsumeBlocks(ctx, outCh, 0, maxBlockNumber) | ||
require.Error(t, err) // this is expected | ||
if tc.BlockByNumberIsBroken { | ||
require.Equal(t, producedBlockNumber, int64(0)) | ||
} | ||
}) | ||
} | ||
} | ||
|
||
func TestBlockSendingLoop(t *testing.T) { | ||
t.Skip("not implemented") | ||
testcases := []string{ | ||
"we're up to date, following the head", | ||
"we're failing intermittently, the Dune API is broken", | ||
"we're erroring systematically, the Dune API is down", | ||
} | ||
for _, testcase := range testcases { | ||
t.Run(testcase, func(t *testing.T) { | ||
t.Skip("not implemented") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add a TODO? |
||
}) | ||
} | ||
} | ||
|
||
func TestRunLoop(t *testing.T) { | ||
t.Skip("not implemented") | ||
func TestRunLoopBaseCase(t *testing.T) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice |
||
testCases := []struct { | ||
name string | ||
i int64 | ||
}{ | ||
{name: "1 block", i: 1}, | ||
{name: "100 blocks", i: 100}, | ||
} | ||
sentBlockNumber := int64(0) | ||
producedBlockNumber := int64(0) | ||
duneapi := &duneapi_mock.BlockchainIngesterMock{ | ||
SendBlockFunc: func(block models.RPCBlock) error { | ||
atomic.StoreInt64(&sentBlockNumber, block.BlockNumber) | ||
return nil | ||
}, | ||
} | ||
rpcClient := &jsonrpc_mock.BlockchainClientMock{ | ||
LatestBlockNumberFunc: func() (int64, error) { | ||
return 1000, nil | ||
}, | ||
BlockByNumberFunc: func(_ context.Context, blockNumber int64) (models.RPCBlock, error) { | ||
atomic.StoreInt64(&producedBlockNumber, blockNumber) | ||
return models.RPCBlock{ | ||
BlockNumber: blockNumber, | ||
Payload: []byte(`block`), | ||
}, nil | ||
}, | ||
} | ||
for _, tc := range testCases { | ||
t.Run(tc.name, func(t *testing.T) { | ||
ing := ingester.New(slog.New(slog.NewTextHandler(io.Discard, nil)), rpcClient, duneapi, ingester.Config{ | ||
MaxBatchSize: 1, | ||
PollInterval: 1000 * time.Millisecond, | ||
}) | ||
|
||
err := ing.Run(context.Background(), 0, tc.i) | ||
require.NoError(t, err) | ||
require.Equal(t, producedBlockNumber, tc.i) | ||
require.Equal(t, sentBlockNumber, tc.i) | ||
}) | ||
} | ||
} | ||
|
||
func TestRunLoopUntilCancel(t *testing.T) { | ||
ctx, cancel := context.WithCancel(context.Background()) | ||
maxBlockNumber := int64(1000) | ||
sentBlockNumber := int64(0) | ||
producedBlockNumber := int64(0) | ||
duneapi := &duneapi_mock.BlockchainIngesterMock{ | ||
SendBlockFunc: func(block models.RPCBlock) error { | ||
atomic.StoreInt64(&sentBlockNumber, block.BlockNumber) | ||
if block.BlockNumber == maxBlockNumber { | ||
// cancel execution when we send the last block | ||
cancel() | ||
} | ||
return nil | ||
}, | ||
} | ||
rpcClient := &jsonrpc_mock.BlockchainClientMock{ | ||
LatestBlockNumberFunc: func() (int64, error) { | ||
return maxBlockNumber + 1, nil | ||
}, | ||
BlockByNumberFunc: func(_ context.Context, blockNumber int64) (models.RPCBlock, error) { | ||
atomic.StoreInt64(&producedBlockNumber, blockNumber) | ||
return models.RPCBlock{ | ||
BlockNumber: blockNumber, | ||
Payload: []byte(`block`), | ||
}, nil | ||
}, | ||
} | ||
ing := ingester.New(slog.New(slog.NewTextHandler(io.Discard, nil)), rpcClient, duneapi, ingester.Config{ | ||
MaxBatchSize: 1, | ||
PollInterval: 1000 * time.Millisecond, | ||
}) | ||
|
||
err := ing.Run(ctx, 0, maxBlockNumber) | ||
require.ErrorIs(t, err, context.Canceled) | ||
require.Equal(t, producedBlockNumber, maxBlockNumber) | ||
require.Equal(t, sentBlockNumber, maxBlockNumber) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't realize
/* */
was valid in Go!