Skip to content

Fix/order acceptance consistency - #693

Open
fatemeh-i wants to merge 7 commits into
devfrom
fix/order-acceptance-consistency
Open

Fix/order acceptance consistency#693
fatemeh-i wants to merge 7 commits into
devfrom
fix/order-acceptance-consistency

Conversation

@fatemeh-i

Copy link
Copy Markdown
Member

No description provided.

AmirRajabii
AmirRajabii previously approved these changes Jul 26, 2026
val current = orderBooks[pairKey]

if (current !== expected) {
return false

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.

Should we here throw exception or return false? in OrderCommandProcessor#L53 the output of replace is discarded and recovery is only triggered when an exception is throw.

prepared.stateTransition?.let { transition -> try { orderBookStore.replace( pairKey = pairKey, expected = currentBook, replacement = transition.preparedBook ) } catch (exception: Throwable) { recoveryManager.enterRecovery( cause = exception, pair = command.pair, expectedSequence = transition.nextSequence ) ...

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

My bad, you are right

prepared: PreparedCommandResult,
inputMetadata: InputKafkaMetadata
) {
check(kafkaTemplate.isTransactional) {

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.

Do we have a transactional KafkaTemplate bean? As I can see in EventsKafkaConfig#L25 that no "transaction.id" config is passed.

@fatemeh-i fatemeh-i Jul 28, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

You are right, I forgot to add my changed configuration file : ProducerConfig.ENABLE_IDEMPOTENCE_CONFIG to true,
ProducerConfig.RETRIES_CONFIG to 10,
ProducerConfig.DELIVERY_TIMEOUT_MS_CONFIG to 5000,
ProducerConfig.REQUEST_TIMEOUT_MS_CONFIG to 5000,
ProducerConfig.TRANSACTIONAL_ID_CONFIG to "matching-engine-tx"

val afterSnapshot =
workingBook.snapshot()

// We will implement this part next.

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.

What is this missing next step?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The missing next step is to compare the before and after order book snapshots and generate a delta containing only the changed state, instead of publishing the complete after-snapshot for every command. This optimization should reduce message size and improve Kafka/network performance. It has been intentionally postponed so the current full-snapshot approach can first be validated in a simpler and more reliable form.

logCurrentState()
}

override fun handleEditCommand(orderCommand: OrderEditCommand): Order? {

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.

It seems handleEditCommand is removed, how can handle editing of an order?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Our business team has suggested that, from a user-experience perspective, exchanges should use a cancel + create new order flow instead of editing an existing order. We are still discussing and evaluating this decision with the team.

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

Successfully merging this pull request may close these issues.

3 participants