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

Emit events when replaying external txns #417

Merged
merged 11 commits into from
Aug 18, 2023
Merged

Conversation

pmconne
Copy link
Member

@pmconne pmconne commented Aug 15, 2023

See iTwin/itwinjs-core#5867
itwinjs-core: https://github.com/iTwin/itwinjs-core/tree/pmc/external-txn-events
imodel-native-internal: https://github.com/iTwin/imodel-native-internal/tree/pmc/external-txn-events

TxnManager::ModelChanges seemed to assume changes can't be applied to a read-only briefcase. That's certainly not the case when the watchForChanges flag is enabled when opening the briefcase.
As a result, it would fail to update a model's range index when its geometry changed.

@pmconne pmconne marked this pull request as ready for review August 16, 2023 14:59
@pmconne
Copy link
Member Author

pmconne commented Aug 16, 2023

/azp run imodel-native

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@pmconne
Copy link
Member Author

pmconne commented Aug 16, 2023

@swbsi or @tcobbs-bentley can you please submit a PR to fix or reduce the android build failures?

Execution failed for task ':app:mergeReleaseNativeDebugMetadata'.
> A failure occurred while executing com.android.build.gradle.internal.tasks.MergeNativeDebugMetadataTask$MergeNativeDebugMetadataWorkAction
   > Java heap space

@tcobbs-bentley
Copy link
Member

@swbsi or @tcobbs-bentley can you please submit a PR to fix or reduce the android build failures?

It looks like it's already set to use 4GB. We could of course bump that up, but I don't know if the error is because it's going over 4GB or if 4GB is not available. @swbsi, what do you think?

Note that on my Mac, I have never had an Android build fail when Gradle is configured to use 4GB.

@swbsi
Copy link
Contributor

swbsi commented Aug 16, 2023

@swbsi or @tcobbs-bentley can you please submit a PR to fix or reduce the android build failures?

It looks like it's already set to use 4GB. We could of course bump that up, but I don't know if the error is because it's going over 4GB or if 4GB is not available. @swbsi, what do you think?

Note that on my Mac, I have never had an Android build fail when Gradle is configured to use 4GB.

I have also never had an issue at 4gb locally.

Not sure if the error is really a system oom situation…maybe try a really large value locally (that you know will exceed physical+virtual on your box) and compare the resulting error?

@tcobbs-bentley
Copy link
Member

Not sure if the error is really a system oom situation…maybe try a really large value locally (that you know will exceed physical+virtual on your box) and compare the resulting error?

I'm not sure I'm willing to try doing that. What if we change it from 4GB to 3GB?

@pmconne
Copy link
Member Author

pmconne commented Aug 16, 2023

@swbsi @tcobbs-bentley here is a better place to discuss and track.

@pmconne
Copy link
Member Author

pmconne commented Aug 18, 2023

/azp run imodel-native

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@pmconne
Copy link
Member Author

pmconne commented Aug 18, 2023

/azp run imodel-native

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@pmconne
Copy link
Member Author

pmconne commented Aug 18, 2023

@DanRod1999 @aruniverse is this supposed to show my linked PR branches?

image

@DanRod1999
Copy link
Contributor

DanRod1999 commented Aug 18, 2023

@DanRod1999 @aruniverse is this supposed to show my linked PR branches?

No. This only shows the initial branch that is pulled in by the pipeline as a repository resource. Your linked branch would be shown later inside the actual pipeline run on imodel-native-internal here:
image

@pmconne pmconne merged commit 6b16329 into main Aug 18, 2023
@pmconne pmconne deleted the pmc/external-txn-events branch August 18, 2023 20:24
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.

6 participants