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

SQL Server LastBlockProcessed is set to NVARCHAR #19

Open
jpmyburghbc opened this issue Nov 12, 2022 · 2 comments
Open

SQL Server LastBlockProcessed is set to NVARCHAR #19

jpmyburghbc opened this issue Nov 12, 2022 · 2 comments

Comments

@jpmyburghbc
Copy link

jpmyburghbc commented Nov 12, 2022

The following method does a MaxAsync on the BlockProgress table but the LastBlockProcessed column is set to NVARCHAR in the SQL scripts supplied, as a result, the query will return a value to the nearest 9, 99, 999 etc and not the actual last block processed, this can cause the processor to re-process a large number of blocks when restarted.

public async Task<BigInteger?> GetLastBlockNumberProcessedAsync()

public async Task<BigInteger?> GetLastBlockNumberProcessedAsync() { using (var context = _contextFactory.CreateContext()) { var max = await context.BlockProgress.MaxAsync(b => b.LastBlockProcessed).ConfigureAwait(false); return string.IsNullOrEmpty(max) ? (BigInteger?)null : BigInteger.Parse(max); } }

Steps to recreate

  1.    Add 19999 blocks in the `BlockProgress` table
    
  2.    Now run `SELECT Max([LastBlockProcessed]) FROM [BlockProgress]`
    
  3.    Result returned 9999 as LastBlockProcessed resulting in 10k blocks needing to be reprocessed
    
  4.    Expected query to return 19999 as LastBlockProcessed
    
@juanfranblanco
Copy link
Member

Thanks for this.. this was changed to NVARCHAR as blockNumbers are bigIntegers, but this progress repository (even if simple) should not be storing more than one row at a time..

Maybe adding something like this will be better ->

 var oldEntries = await context.BlockProgress.Where(x => x.LastBlockProcessed != blockEntity.LastBlockProcessed).ToListAsync().ConfigureAwait(false);
                    context.BlockProgress.RemoveRange(oldEntries);
                    await context.SaveChangesAsync().ConfigureAwait(false);

public async Task UpsertProgressAsync(BigInteger blockNumber)

@jpmyburghbc

@jpmyburghbc
Copy link
Author

Thank you for taking the time to respond. I understand the usage better now.

Your recommended fix will help keep the table clean, and that way you can also utilise a SingleOrDefault()/FirstOfDefault() instead of MaxAsync() on GetLastBlockNumberProcessedAsync() which will be better on an NVARCHAR column.

I guess another alternative is to set that SQL column to bigint on in the SQL script, then the MaxAsync() will work as expected. I am only looking at the EF implementation so it's probably not an option on the other processors.

Either way, great work on this, Nethereum is doing great work for the crypto community, we are using it on Quorum and it's working great! Feel free to close the issue if you do not see an action item on this.

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

No branches or pull requests

2 participants