Skip to content

PHOENIX-1674 Snapshot isolation transaction support through Tephra - #129

Closed
twdsilva wants to merge 0 commit into
apache:masterfrom
twdsilva:master
Closed

PHOENIX-1674 Snapshot isolation transaction support through Tephra#129
twdsilva wants to merge 0 commit into
apache:masterfrom
twdsilva:master

Conversation

@twdsilva

Copy link
Copy Markdown
Contributor

@JamesRTaylor@samarthjain

PR will code review feedback.

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.

How is this sleep helping? Is it account for the async nature of the API? FWIW, the admin.createTable() already retries before it throws TableNotFoundException. If the number of retries is too small then you could possibly increasing the number of retries too.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

After we re-create the index htable, we wait till the index become ACTIVE again, not sure what process is responsible for enable/disabling the index state.

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.

This sleep was already there, though, right?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Yes it was.

@JamesRTaylor

Copy link
Copy Markdown
Contributor

I can't find PhoenixTransactionalIndexer. Did your latest pull forget to add new files? Or maybe I'm just missing it? FYI, I remember there are a few bits of commented out code that should be removed from that file. Everything else looks good to me - except for a few minor comments above.

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.

Remove this TODO.

Sign up for freeto 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

@twdsilva@JamesRTaylor@samarthjain