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

Confusing log messages about POLARIS_SEQ #621

Open
snazy opened this issue Jan 7, 2025 · 14 comments
Open

Confusing log messages about POLARIS_SEQ #621

snazy opened this issue Jan 7, 2025 · 14 comments
Labels
bug Something isn't working

Comments

@snazy
Copy link
Member

snazy commented Jan 7, 2025

Describe the bug

The log messages

INFO  [2025-01-07 10:14:25,982 - 1046  ] [main] [] o.a.p.e.p.i.e.PolarisSequenceUtil: Checking if the sequence POLARIS_SEQ exists 
INFO  [2025-01-07 10:14:25,990 - 1054  ] [main] [] o.a.p.e.p.i.e.PolarisSequenceUtil: POLARIS_SEQ does not exist, skipping NEXTVAL 

are just confusing - the code mentions "legacy", but there is no "legacy" in Polaris.

To Reproduce

No response

Actual Behavior

No response

Expected Behavior

No response

Additional context

No response

System information

No response

@snazy snazy added the bug Something isn't working label Jan 7, 2025
@eric-maynard
Copy link
Contributor

Sorry, what are you confused by?

@snazy
Copy link
Member Author

snazy commented Jan 11, 2025

Because there is no "legacy" in Polaris - and that message implies that there's something missing. TL;DR the message is not useful for users, right?

@eric-maynard
Copy link
Contributor

eric-maynard commented Jan 11, 2025 via email

@snazy
Copy link
Member Author

snazy commented Jan 14, 2025

What do you mean by “there is no legacy”?

There's no Polaris release yet, no?

@snazy
Copy link
Member Author

snazy commented Jan 14, 2025

Also, what can or should a user do when he sees those messages?

@eric-maynard
Copy link
Contributor

Also, what can or should a user do when he sees those messages?

Be aware of what entity in the backend is being used.

There's no Polaris release yet, no?

I'm not sure I follow you here. The message doesn't refer to any version.

@snazy
Copy link
Member Author

snazy commented Jan 17, 2025

Be aware of what entity in the backend is being used.

@eric-maynard the term "legacy" is used in the Javadoc: "If the legacy POLARIS_SEQ generator is available it will be used then cleaned up. In all other cases the POLARIS_SEQUENCE table is used directly.". No previous version (there is none released, right?) of the Apache Polaris code base used that identifier.

@eric-maynard
Copy link
Contributor

I can see the Javadoc, but I don't see what you're confused by.

@eric-maynard
Copy link
Contributor

Are you saying that you think "legacy" usually is an identifier used to refer to a particular version of software? It is not.

@snazy
Copy link
Member Author

snazy commented Jan 17, 2025

In this case it does. Again, there is no "POLARIS_SEQ legacy" in this code base, so the whole thing can and should go away. It is confusing.

@eric-maynard
Copy link
Contributor

No, in this case it actually doesn't. Maybe that's the source of confusion here?

Older "versions" of Polaris used this entity POLARIS_SEQ, but it isn't used by newer "versions". It is a sort of vestigial entity that may be used one-time to continue the sequence, and legacy is used in that sense.

Is there another term that you think we should use which would be more clear?

@snazy
Copy link
Member Author

snazy commented Jan 17, 2025

The whole point is that a) the logging is confusing and the "legacy" is not properly defined. But still, I cannot find any usage of POLARIS_SEQ even in the initial commit.

@eric-maynard
Copy link
Contributor

eric-maynard commented Jan 17, 2025

Yes -- if POLARIS_SEQ (removed here) was used in a release then I would prefer this comment to say something like:

If the generator `POLARIS_SEQ` used prior to Polaris X.Y is...

But since there isn't a version, "legacy" was the most descriptive term I could think of. The generator is legacy in that it may still be around in the metastore but should only be used in a one-off operation and otherwise is disused now.

Eventually I think we can even clean a lot of this up. That is, remove the logging and a lot of the code. We just needed this to make sure we didn't break any existing deployments using the old (legacy) metastore implementation.

The idea was to wait to ensure nobody was still relying on POLARIS_SEQ before ripping this code out. That may be the case by now.

@snazy
Copy link
Member Author

snazy commented Jan 17, 2025

TBH: since there is no Polaris release yet, there is nothing to care about. I'll start a dev-ML discussion about this.
TL;DR the whole POLARIS_SEQ stuff can and should go away, especially the user-nag

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants