Skip to content

[loadgen] LoadGen without committer#70

Merged
cendhu merged 1 commit into
hyperledger:mainfrom
liran-funaro:loadgen-orderer-batch
Jul 8, 2025
Merged

[loadgen] LoadGen without committer#70
cendhu merged 1 commit into
hyperledger:mainfrom
liran-funaro:loadgen-orderer-batch

Conversation

@liran-funaro

@liran-funaro liran-funaro commented Jul 7, 2025

Copy link
Copy Markdown
Contributor

Type of change

  • New feature
  • Improvement (improvement to code, performance, etc)

Description

  • Pipeline envelope serialization
  • Orderer client without a sidecar

Related issues

Signed-off-by: Liran Funaro <liran.funaro@gmail.com>
@liran-funaro liran-funaro requested a review from cendhu July 7, 2025 13:28
@liran-funaro liran-funaro added enhancement New feature or request loadgen labels Jul 7, 2025
@cendhu cendhu requested a review from Copilot July 8, 2025 07:41

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull Request Overview

Adds the ability to run the LoadGen client directly against an orderer without a committer sidecar, refactors envelope creation, and unifies delivery receiver logic.

  • Extracted CreateEnvelope from SendWithEnv to allow envelope serialization without immediately sending.
  • Introduced a new TestLoadGenForOnlyOrderer and corresponding CLI/template support for the “only orderer” mode.
  • Refactored sidecar and orderer delivery into a shared runDeliveryReceiver helper.

Reviewed Changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
utils/broadcastdeliver/envelope_creator.go Extracted CreateEnvelope method and updated SendWithEnv.
loadgen/client_test.go Added TestLoadGenForOnlyOrderer.
loadgen/adapters/sidecar_receiver.go Renamed receiver types/functions and refactored into runDeliveryReceiver.
loadgen/adapters/orderer.go Switched to unified delivery receiver and introduced mapToBatch/sendBatch.
loadgen/adapters/config.go Clarified SidecarEndpoint documentation.
integration/test/loadgen_test.go Added integration scenario for “only orderer” mode.
integration/runner/runtime.go Introduced LoadGenForOnlyOrderer flag and template switch.
cmd/config/templates/loadgen_client_orderer.yaml Reduced default broadcast parallelism.
cmd/config/templates/loadgen_client_only_orderer.yaml Added new template for only-orderer client.
cmd/config/create_config_file.go Embedded the new only-orderer template.
Comments suppressed due to low confidence (4)

loadgen/adapters/sidecar_receiver.go:36

  • [nitpick] Minor grammar: change to // runSidecarReceiver starts receiving blocks from the sidecar. for consistency.
// runSidecarReceiver start receiving blocks from the sidecar.

cmd/config/create_config_file.go:97

  • [nitpick] The variable name TemplateLoadGenOnlyOrderer omits Client but uses templateLoadGenOnlyOrdererClient internally; consider renaming for consistency.
	TemplateLoadGenOnlyOrderer              = templateLoadGenOnlyOrdererClient + templateLoadGenCommon

loadgen/adapters/sidecar_receiver.go:34

  • [nitpick] The name statusIdx is not descriptive; consider renaming to something like txFilterMetadataIndex for clarity.
const statusIdx = int(common.BlockMetadataIndex_TRANSACTIONS_FILTER)

loadgen/adapters/sidecar_receiver.go:147

  • The function is returning statusBatch (a slice) but its callers expect an error; this signature mismatch will not compile or behave as intended.
	return statusBatch

Comment thread utils/broadcastdeliver/envelope_creator.go
Comment thread loadgen/adapters/sidecar_receiver.go

@cendhu cendhu left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM. I have a few suggestions which can be addressed in separate PRs as they are out of scope for this one.

txIDs := make([]string, len(txs))
for i, tx := range txs {
var err error
envs[i], txIDs[i], err = stream.CreateEnvelope(tx)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We have an issue to parallelize this. This change may make it hard to achieve that.

})
}
processedBlocks.Write(statusBatch)
processedBlocks.Write(mapToStatusBatch(block))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We can use multiple goroutines to process the committed status as we are unmarshaling the envelope. Otherwise, this could increase the latency unnecessarily.

@cendhu cendhu merged commit 7c06fc4 into hyperledger:main Jul 8, 2025
9 checks passed
@liran-funaro liran-funaro deleted the loadgen-orderer-batch branch July 9, 2025 07:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request loadgen

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[loadgen] Add no-committer support [loadgen] pipline orderer env serialization

3 participants