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

Jump to bottom

Implement panic recovery in HandleGetStateByRange #5084

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Implement panic recovery in HandleGetStateByRange #5084

wants to merge 1 commit into from

Conversation

dviejokfs
Copy link

Type of change

  • Improvement (improvement to code, performance, etc)

Description

This pull request introduces several improvements in the HandleGetStateByRange function within the core/chaincode/handler.go file and the GetStateRangeScanIteratorWithPagination function within the core/ledger/kvledger/txmgmt/statedb/stateleveldb/stateleveldb.go file. The changes include:

  • Adding panic recovery in the HandleGetStateByRange function to log errors and clean up any query contexts that may have been created.
  • Validating start and end keys to ensure they are not both empty.
  • Adding validation for the page size to ensure it is greater than zero.
  • Cleaning up query contexts if errors occur during the range query or if the range query iterator is nil.
  • Wrapping the iterator creation in a recover block in GetStateRangeScanIteratorWithPagination to handle potential panics and return meaningful error messages.
  • Ensuring that a nil iterator is not returned from the database.

Additional details

These changes help improve the robustness of the code by handling potential panics and validating inputs, which can help prevent unexpected errors and improve the overall stability of the system.

Related issues

#5001

@dviejokfs
Implement panic recovery in HandleGetStateByRange to ensure cleanup o…
893a2f5 
…f query context. Add test case to verify recovery behavior during panic scenarios.

Signed-off-by: David VIEJO <[email protected]>
@dviejokfs dviejokfs requested a review from a team as a code owner December 20, 2024 13:18
@dviejokfs dviejokfs mentioned this pull request Dec 20, 2024
Closed
@pfi79
Copy link
Contributor

pfi79 commented Dec 20, 2024

@dviejokfs I really want to get to the bottom of the error.

  1. in your panic stacktrace I don't see that the code goes through the place you corrected.

  2. the error can be either in the goleveldb library or in incorrect parameters passed to the GetStateByRange method (maybe they are overwritten somewhere along the way, then it's an error in the fabric).
    What is the best way to separate one from the other?
    If it is an error in goleveldb, then this is not the only place to insert recover.

I suggest you to insert logs in the place where you make changes and logs in the call in the chaincode. And check that the input parameters in GetStateByRange are correct and do not change along the whole path.

@dviejokfs
Copy link
Author

@pfi79 Here you go


2024-12-20T12:06:04.629+01:00 panic: runtime error: slice bounds out of range [48:14]
2024-12-20T12:06:04.629+01:00 
2024-12-20T12:06:04.629+01:00 goroutine 17810 [running]:
2024-12-20T12:06:04.629+01:00 github.com/syndtr/goleveldb/leveldb.tFiles.newIndexIterator({0xc005b5c000?, 0x7a3?, 0x7a3}, 0xc00238b8f0, 0xc001bfc940, 0xc000572270, 0xc0004ca420)
2024-12-20T12:06:04.629+01:00 	/vendor/github.com/syndtr/goleveldb/leveldb/table.go:308 +0x325
2024-12-20T12:06:04.629+01:00 github.com/syndtr/goleveldb/leveldb.(*version).getIterators(0xc003a2fec0, 0x26f5108?, 0xc0004ca420)
2024-12-20T12:06:04.629+01:00 	/vendor/github.com/syndtr/goleveldb/leveldb/version.go:269 +0x135
2024-12-20T12:06:04.629+01:00 github.com/syndtr/goleveldb/leveldb.(*DB).newRawIterator(0xc0003ef180, 0x0, {0x0, 0x0, 0x1?}, 0xc000572270, 0xc0004ca420)
2024-12-20T12:06:04.629+01:00 	/vendor/github.com/syndtr/goleveldb/leveldb/db_iter.go:41 +0xfb
2024-12-20T12:06:04.629+01:00 github.com/syndtr/goleveldb/leveldb.(*DB).newIterator(0xc0003ef180, 0xc000100800?, {0x0?, 0x0?, 0x0?}, 0x152005fd, 0xc0095b8850?, 0xc0004ca420)
2024-12-20T12:06:04.629+01:00 	/vendor/github.com/syndtr/goleveldb/leveldb/db_iter.go:79 +0x2db
2024-12-20T12:06:04.629+01:00 github.com/syndtr/goleveldb/leveldb.(*DB).NewIterator(0xc0003ef180, 0x18?, 0x18a5cc0?)
2024-12-20T12:06:04.629+01:00 	/vendor/github.com/syndtr/goleveldb/leveldb/db.go:897 +0x134
2024-12-20T12:06:04.629+01:00 github.com/hyperledger/fabric/common/ledger/util/leveldbhelper.(*DB).GetIterator(0x0?, {0xc00958c1c0?, 0x0?, 0xc000120080?}, {0xc0076c99e0?, 0x1b58927?, 0x28?})
2024-12-20T12:06:04.629+01:00 	/common/ledger/util/leveldbhelper/leveldb_helper.go:159 +0x137
2024-12-20T12:06:04.629+01:00 github.com/hyperledger/fabric/common/ledger/util/leveldbhelper.(*DBHandle).GetIterator(0xc0006c9320, {0xc008ec0a30, 0xe, 0xc0095b89f0?}, {0xc003e142d0, 0x47, 0x200000003?})
2024-12-20T12:06:04.629+01:00 	/common/ledger/util/leveldbhelper/leveldb_provider.go:297 +0x371
2024-12-20T12:06:04.629+01:00 github.com/hyperledger/fabric/core/ledger/kvledger/txmgmt/statedb/stateleveldb.(*versionedDB).GetStateRangeScanIteratorWithPagination(0xc0037e90f8, {0xc00519bd90, 0xb}, {0x269c788, 0x1}, {0xc00ac51b40, 0x3a}, 0xa)
2024-12-20T12:06:04.629+01:00 	/core/ledger/kvledger/txmgmt/statedb/stateleveldb/stateleveldb.go:167 +0x478
2024-12-20T12:06:04.629+01:00 github.com/hyperledger/fabric/core/ledger/kvledger/txmgmt/txmgr.newResultsItr({0xc00519bd90, 0xb}, {0x269c788, 0x1}, {0xc00ac51b40, 0x3a}, 0x5?, {0x1d351a8?, 0xc0037e9188?}, 0xc0061f7800, ...)
2024-12-20T12:06:04.629+01:00 	/core/ledger/kvledger/txmgmt/txmgr/query_executor.go:410 +0x92
2024-12-20T12:06:04.629+01:00 github.com/hyperledger/fabric/core/ledger/kvledger/txmgmt/txmgr.(*queryExecutor).GetStateRangeScanIteratorWithPagination(0xc00945ff80, {0xc00519bd90?, 0x1869b80?}, {0x269c788?, 0xc0095b8c88?}, {0xc00ac51b40?, 0xc008ec0a10?}, 0x10?)
2024-12-20T12:06:04.629+01:00 	/core/ledger/kvledger/txmgmt/txmgr/query_executor.go:151 +0x15f
2024-12-20T12:06:04.629+01:00 github.com/hyperledger/fabric/core/ledger/kvledger/txmgmt/txmgr.(*txSimulator).GetStateRangeScanIteratorWithPagination(0xc008ec0a08?, {0xc00519bd90?, 0x8?}, {0x269c788?, 0xc00392aaf0?}, {0xc00ac51b40?, 0xab292c?}, 0x19dbca0?)
2024-12-20T12:06:04.629+01:00 	/core/ledger/kvledger/txmgmt/txmgr/tx_simulator.go:183 +0x10e
2024-12-20T12:06:04.629+01:00 github.com/hyperledger/fabric/core/chaincode.(*Handler).HandleGetStateByRange(0xc000373320, 0xc00411e700, 0xc0091170e0)
2024-12-20T12:06:04.629+01:00 	/core/chaincode/handler.go:756 +0x296
2024-12-20T12:06:04.629+01:00 github.com/hyperledger/fabric/core/chaincode.(*Handler).HandleTransaction(0xc000373320, 0xc00411e700, 0xc006b6c090)
2024-12-20T12:06:04.629+01:00 	/core/chaincode/handler.go:253 +0x373
2024-12-20T12:06:04.629+01:00 created by github.com/hyperledger/fabric/core/chaincode.(*Handler).handleMessageReadyState in goroutine 212
2024-12-20T12:06:04.629+01:00 	/core/chaincode/handler.go:195 +0x3e9

@dviejokfs
Copy link
Author

@pfi79
do you have linkedin/discord to discuss there?

@pfi79
Copy link
Contributor

pfi79 commented Dec 20, 2024

do you have linkedin/discord to discuss there?

it's better to discuss it here, so that others can suggest something.

@pfi79
Copy link
Contributor

pfi79 commented Dec 20, 2024

/common/ledger/util/leveldbhelper/leveldb_helper.go:159 +0x137
2024-12-20T12:06:04.629+01:00 github.com/hyperledger/fabric/common/ledger/util/leveldbhelper.(*DBHandle).GetIterator(0xc0006c9320, {0xc008ec0a30, 0xe, 0xc0095b89f0?}, {0xc003e142d0, 0x47, 0x200000003?})
2024-12-20T12:06:04.629+01:00 /common/ledger/util/leveldbhelper/leveldb_provider.go:297 +0x371
2024-12-20T12:06:04.629+01:00 github.com/hyperledger/fabric/core/ledger/kvledger/txmgmt/statedb/stateleveldb.(*versionedDB).GetStateRangeScanIteratorWithPagination(0xc0037e90f8, {0xc00519bd90, 0xb}, {0x269c788, 0x1}, {0xc00ac51b40, 0x3a}, 0xa)
2024-12-20T12:06:04.629+01:00 /core/ledger/kvledger/txmgmt/statedb/stateleveldb/stateleveldb.go:167 +0x478
2024-12-20T12:06:04.629+01:00 github.com/hyperledger/fabric/core/ledger/kvledger/txmgmt/txmgr.newResultsItr({0xc00519bd90, 0xb}, {0x269c788, 0x1}, {0xc00ac51b40, 0x3a}, 0x5?, {0x1d351a8?, 0xc0037e9188?}, 0xc0061f7800, ...)
2024-12-20T12:06:04.629+01:00 /core/ledger/kvledger/txmgmt/txmgr/query_executor.go:410 +0x92
2024-12-20T12:06:04.629+01:00 github.com/hyperledger/fabric/core/ledger/kvledger/txmgmt/txmgr.(*queryExecutor).GetStateRangeScanIteratorWithPagination(0xc00945ff80, {0xc00519bd90?, 0x1869b80?}, {0x269c788?, 0xc0095b8c88?}, {0xc00ac51b40?, 0xc008ec0a10?}, 0x10?)
2024-12-20T12:06:04.629+01:00 /core/ledger/kvledger/txmgmt/txmgr/query_executor.go:151 +0x15f
2024-12-20T12:06:04.629+01:00 github.com/hyperledger/fabric/core/ledger/kvledger/txmgmt/txmgr.(*txSimulator).GetStateRangeScanIteratorWithPagination(0xc008ec0a08?, {0xc00519bd90?, 0x8?}, {0x269c788?, 0xc00392aaf0?}, {0xc00ac51b40?, 0xab292c?}, 0x19dbca0?)
2024-12-20T12:06:04.629+01:00 /core/ledger/kvledger/txmgmt/txmgr/tx_simulator.go:183 +0x10e

It's a stacktrace of what version of fabric.

@dviejokfs
Copy link
Author

@pfi79 2.5.5

@pfi79
Copy link
Contributor

pfi79 commented Dec 20, 2024

Type of change

  • Improvement (improvement to code, performance, etc)

Description

This pull request introduces several improvements in the HandleGetStateByRange function within the core/chaincode/handler.go file and the GetStateRangeScanIteratorWithPagination function within the core/ledger/kvledger/txmgmt/statedb/stateleveldb/stateleveldb.go file. The changes include:

  • Adding panic recovery in the HandleGetStateByRange function to log errors and clean up any query contexts that may have been created.
  • Validating start and end keys to ensure they are not both empty.
  • Adding validation for the page size to ensure it is greater than zero.
  • Cleaning up query contexts if errors occur during the range query or if the range query iterator is nil.
  • Wrapping the iterator creation in a recover block in GetStateRangeScanIteratorWithPagination to handle potential panics and return meaningful error messages.
  • Ensuring that a nil iterator is not returned from the database.

Additional details

These changes help improve the robustness of the code by handling potential panics and validating inputs, which can help prevent unexpected errors and improve the overall stability of the system.

Related issues

#5001

If the goleveldb library is indeed the culprit, I would recover add as close to the library itself as possible. For example, here somewhere /common/ledger/util/leveldbhelper/leveldb_helper.go:159

And not only in the iterator function itself, but in other functions as well, so that working with this library doesn't add panic somewhere else.

@dviejokfs
Copy link
Author

@pfi79 so you would add it in the functions for the file common/ledger/util/leveldbhelper/leveldb_helper.go?

@pfi79
Copy link
Contributor

pfi79 commented Dec 20, 2024

so you would add it in the functions for the file common/ledger/util/leveldbhelper/leveldb_helper.go?

I'd like to start by clarifying the cause of the panic.

@denyeart
Copy link
Contributor

I agree with @pfi79 , you are very close, might as well complete the analysis and find the exact root cause so that we know it is handled completely. In addition to more logging in Fabric, you can add more logging in goleveldb in your vendor directory before building the peer. Then compare the log entries when it is working versus when it is not working.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@dviejokfs @pfi79 @denyeart