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

Es search order field #2241

Merged
merged 8 commits into from
Aug 28, 2020
Merged

Es search order field #2241

merged 8 commits into from
Aug 28, 2020

Conversation

ccorcoveanu
Copy link
Contributor

@ccorcoveanu ccorcoveanu commented Aug 26, 2020

Added new field to be indexed in elastic. This helps the front end using the "search_after" parameter to scroll deeper than 10k elements.

@ccorcoveanu ccorcoveanu marked this pull request as ready for review August 27, 2020 13:27
@miiu96 miiu96 self-requested a review August 27, 2020 17:11
@@ -248,6 +251,33 @@ func (tdp *txDatabaseProcessor) groupNormalTxsAndRewards(
return transactions, rewardsTxs
}

func (tdp *txDatabaseProcessor) setTransactionSearchOrder(transactions map[string]*Transaction, shardId uint32) map[string]*Transaction {
currentOrder := 0
shardIdentifier := tdp.createShardIdentifier(shardId)
Copy link
Contributor

Choose a reason for hiding this comment

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

shardID

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ShardID is the parameter passed to the function. I used "identifier" to: 1. avoid name collision and 2. to avoid confusion between an actual shard id in Elrond context and this custom identifier

currentOrder := 0
shardIdentifier := tdp.createShardIdentifier(shardId)

for _, tx := range transactions {
Copy link
Contributor

Choose a reason for hiding this comment

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

The iteration is done over a map. This will not ensure the same order each time. Does it matter?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are right. But we don't care. The only thing we are interested in here is to have a tie breaker for the front end where the shard id is the same and the (round) timestamp is the same

Copy link
Contributor

Choose a reason for hiding this comment

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

ok

shardIdentifier := tdp.createShardIdentifier(shardId)

for _, tx := range transactions {
stringOrder := fmt.Sprintf("%d%d", shardIdentifier, currentOrder)
Copy link
Contributor

Choose a reason for hiding this comment

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

The 10th tx of metachain will have the order 110 which will be higher than the 9th tx of the shard 2 which will have the order 49. Does it matter for the logic that you needed from this method?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's right but we don't care about that. We already can have a tie breaker and sort by shard id. We only need to have a rank between items in the same shard and the same indexed bulk

Copy link
Contributor

Choose a reason for hiding this comment

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

ok


func (esd *elasticSearchDatabase) computeBlockSearchOrder(header data.HeaderHandler) uint32 {
shardIdentifier := esd.txDatabaseProcessor.createShardIdentifier(header.GetShardID())
stringOrder := fmt.Sprintf("%d%d", shardIdentifier, header.GetNonce())
Copy link
Contributor

Choose a reason for hiding this comment

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

The 10th nonce of metachain will have the order 110 which will be higher than the 9th nonce of the shard 2 which will have the order 49. Does it matter for the logic that you needed from this method?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's right but we don't care about that. We already can have a tie breaker and sort by shard id. We only need to have a rank between items in the same shard and the same indexed bulk

Copy link
Contributor

Choose a reason for hiding this comment

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

ok

Copy link
Contributor

@LucianMincu LucianMincu left a comment

Choose a reason for hiding this comment

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

System tests passed.

@LucianMincu LucianMincu merged commit b6e94e6 into development Aug 28, 2020
@LucianMincu LucianMincu deleted the es-search-order-field branch August 28, 2020 10:08
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.

None yet

4 participants